Repository navigation
Premium Analytics: describe the applied date configuration in the section header subtitle - #50897
Conversation
the subtitle has no source until the date-config formatter lands
scroll moves to the page content so tabs scroll away with it
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 5 files.
4 files are newly checked for coverage.
|
Add getDateRangeSpan and formatDateRangeLong. The span is derived from the range, not the preset that produced it, so a window stepped off a preset still reports its own length.
Surfaces describing what the widgets show need the committed comparison, not the staged one: a comparison change rides along uncommitted when a primary edit is already open.
Adds getSectionSubtitle, which frames the range, its length, and any active comparison as translatable copy. The story now derives it from applied state instead of a static arg.
Feeds the subtitle from the applied range and comparison, so it holds still while a range edit is staged and moves only on Apply.
A day-aligned single day was repeating the same endpoint twice, and a rolling 24-hour window named two calendar days it only partly covers.
da653f4 to
bc59fa1
Compare
The applied comparison rebuilt hasComparisonEnabled inline and dropped its trim, so a whitespace compare_from would announce a comparison the report params reject.
The long range formatter spelled out the pattern for DATE_FORMATS.full and its year-less variant. Register both as named presets instead.
Three date modules each redeclared it with the same note about the pending sibling-package wiring.
| const handleComparisonChange = useCallback( | ||
| ( _range: DateRange | undefined, nextPresetId?: ComparisonPresetId ) => { | ||
| setComparisonPresetId( nextPresetId ); | ||
| onAppliedChange( { range: committed.range, comparisonPresetId: nextPresetId } ); |
There was a problem hiding this comment.
This does not quite mirror useReportDateFilters. When a primary range edit is already staged, production keeps the comparison change staged as well and does not update the applied subtitle until Apply. Here, onAppliedChange always runs immediately with the committed range.
Should the story track whether the primary state is dirty and defer this applied-state update in that case?
| if ( months ) { | ||
| if ( months >= MIN_MONTHS_FOR_YEARS && months % MONTHS_PER_YEAR === 0 ) { | ||
| return { unit: 'year', value: months / MONTHS_PER_YEAR }; | ||
| } |
There was a problem hiding this comment.
I noticed that the YearSurface story today shows Friday, January 1, 2021 – Thursday, July 30, 2026 (2037 days).
It seems like this is because getWholeMonths needs exact calendar-month alignment, but All time ends at endOfToday. So the unit ends up tracking today's date rather than the range:
| If today is… | All time reads |
|---|---|
| mid-month (most of the year) | (2037 days) |
| the last day of a month | (67 months) |
| Dec 31 | (6 years) |
Last 365 days has a milder version of the same thing. This may require preserving the applied preset ID or defining how partial multi-year ranges should be summarized?
This reverts commit 29193d2.
measured, it tracked today's date instead of the range
defer comparison commit on staged edits, pass the preset
escaped literals and the numeric weekday tokens
an open-ended range measures by the day it is read on
plus a site-zone reference year and a translatable dash
Proposed changes
Closes WOOA7S-1824.
SectionHeadershipped with the subtitle empty because nothing derived it. This fills it in: the applied range spelled out, how long that range is, and the comparison when one is active.The length comes from the range, not the preset
The spec's second example steps back a period and still reads
(7 days), but by then the preset iscustom. SogetDateRangeSpanmeasures the range itself, and the subtitle will keep working once period navigation (WOOA7S-1793's prev/next sub-issue) lands, with no change here.It picks the coarsest unit that divides the range evenly, with two floors that keep the copy honest: a whole month stays in days below two months, so "Last 30 days" does not become "1 month", and years only kick in from two years up, so a twelve-month window keeps reading as
12 monthsthe way the design spells it. Without the second floor the year surface's "All time" would read(72 months).A window of a day or less is named by one date rather than two endpoints, which covers both
today(whose endpoints are the same date) and the rollinglast-24-hours(which straddles two calendar days without being about either in full).Where the pieces live
The issue placed the formatter in
packages/formatters/, but the subtitle is user-facing copy with a plural form and a comparison clause, which belongs next to the component that renders it. So the split follows the boundary the package already draws, the same onegetComparisonTriggerLabelsits on:formattersgetDateRangeSpan,formatDateRangeLonguigetSectionSubtitleSectionHeaderThe subtitle follows applied state
It describes what the widgets are showing, so it must not move while a range edit is staged.
useReportDateFiltersexposed only the staged comparison; this addsappliedComparisonPresetId, read from committed state.That distinction matters in one case: a comparison change normally commits on its own, but it rides along uncommitted when a primary edit is already open. Reading the staged value there would announce a comparison the widgets are not showing yet.
Rebased onto the locale-aware formatter
#50888 landed while this was in review, moving date formatting from
date-fnspatterns to@wordpress/dateand narrowingDateFormatNametomedium | short | year | iso.formatDateRangeLongused three of the names that went away.longwas already whatmediumrenders, so it maps straight across. The two weekday-bearing forms come back as named formats derived from the site'sdate_format, the same wayshortis:withWeekday()inphp-format.tsputs the weekday in front, andfull/fullNoYearpair it with the site format and with its year-stripped variant. No custom-pattern escape hatch is reintroduced.The weekday name still comes from WordPress's translation tables. Only the comma joining it to the date is ours, since core publishes no weekday-bearing format to take one from.
getDateRangeSpanis date arithmetic rather than formatting, so it keeps usingdate-fnsand the dependency goes back on the package. That does not touch the lockfile:pnpm-workspace.yamlglobsprojects/*/*, sopremium-analytics/packages/*arelink:folders rather than pnpm importers.A timezone hazard the swap exposed
Five tests were building dates in the runner's local zone while
dateI18nrenders in the site's, which movedfromback a day. Production is not affected: ranges areTZDates anchored to the site's zone, sodate-fnsday boundaries anddateI18noutput read the same frame. The fixtures now do the same rather than the code being changed to paper over it.format-date.test.tscovers the two new names, including the case where the site's owndate_formatalready names the weekday and must not get a second one.Storybook
The
SectionHeaderstories passed a staticsubtitlearg. They now derive it from applied state, so the Default story demonstrates the staging behavior directly: change the range and the subtitle holds until Apply, pick a comparison and it moves right away.Related product discussion/links
SectionHeader, and rebases onto Premium Analytics: format dates with the site locale instead of hardcoded US patterns #50888, which made date formatting locale-aware. Both have merged.Does this pull request change what data or activity we track or use?
No.
Testing instructions
Setup
Build with
jetpack build packages/premium-analytics --deps, then openwp-admin/admin.php?page=jetpack-premium-analytics.The subtitle
Under each section title, the subtitle states the applied range and its length. With the default
Last 30 days, expect the explicit dates followed by(30 days).Screen.Recording.2026-07-30.at.3.56.19.PM.mov
Check the shape across presets:
Last 24 hours(24 hours)Last 7 days(7 days)Last 30 days(30 days), not(1 month)Last 90 days(90 days)Last 12 months(12 months)Applied, not staged
Open the date picker and pick a different preset without applying. The subtitle must not move. Apply, and it updates.
Then pick a comparison. It commits on its own, so the subtitle gains
vs. Previous periodimmediately. Repeat with a range edit already staged: the comparison rides along, and neither part of the subtitle moves until Apply.Past periods
Hand-edit the URL to a range in a previous year, e.g.
from=2024-07-16T00:00:00.000Z&to=2024-07-22T23:59:59.999Z. The year appears on both dates, so a past window is not mistaken for the current one. A range in the current year omits it.Site locale
Switch the site to a locale that orders dates differently (
es_ESputs the day first) and change Settings → General → Date Format. The subtitle follows both, since it renders through the site's own format rather than a pattern written here. The weekday leads the date in every locale.Storybook
pnpm run storybook, then Packages / Premium Analytics / UI / SectionHeader. The Default story wires the real controls, so the staging behavior above is reproducible there. The YearSurface story shows the multi-year path (All timereads in years).Pre-merge checks
changelog/add-pa-section-header-subtitle)