Repository navigation
Premium Analytics: measure the date presets instead of guessing a breakpoint - #50906
Conversation
russian, dutch and german through real language packs at four container widths
translated separately with context, not truncations of the full label
replaces the fixed 600px compact threshold, which was wrong for every language
own element so the ellipsis can engage; adds a 280px story rung
the fixed widths cannot show whether a boundary settles or flickers
drops containerElement from 12 call sites; every one passed its own wrapper
extracts it from the panel; jsdom has no layout, so the probe stays visual
resolves three report pages where trunk added table deltas next to the removed containerElement
|
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 🤖 🔴 Action required: Please include detailed testing steps, explaining how to test your change, like so: 🔴 Action required: We would recommend that you add a section to the PR description to specify whether this PR includes any changes to data or privacy, like so: Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
tightens the short-label docblock while in there
…-responsive-labels
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 19 files. Only the first 5 are listed here.
1 file is newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
kangzj
left a comment
There was a problem hiding this comment.
Reviewed the diff, ran jp test js packages/premium-analytics (199 suites, green), pnpm run typecheck, and eslint on the changed files — all clean.
Also verified the responsive behavior live in Storybook. Russian at 600px collapses to the abbreviated tier (Last 24H 7D 30D 12M) exactly as described, confirming the per-locale measurement is doing what the PR claims rather than a fixed breakpoint:
One non-blocking nit: the pre-existing CompactLayout story (English @ 360px, untouched by this PR) still carries the docblock "quick presets collapse to the compact select, like mobile layouts." With the new three-tier model, English at 360px now renders the abbreviated pill tier, not the select — the comment (and the story) no longer demonstrate what the name promises:
Worth a follow-up to either narrow the story's width or update its description so it still exercises the select tier. Doesn't block — everything else checks out.
keeps trunk's section header layout; its story still fed the removed containerElement
360px lands on the abbreviated tier, not the select
memoizes the probe, floors the measured width, drops a redundant measure pass
a shrink-to-fit slot made the panel measure its content and never shorten
chihsuan
left a comment
There was a problem hiding this comment.
Great work! @retrofox Overall looks good.
One thing I noticed: the probe and the real trigger can end up measuring two different labels.
After selecting and applying a custom range, clicking a preset pill does not change the custom-range trigger back to “Custom”. It continues to display the previously applied date range.
Also, the probe measures the shorter “Custom” label. This leaves roughly 90px of width unaccounted for, causing the UI to overlap or overflow.
See recording below:
Screen.Recording.2026-08-03.at.1.03.59.PM.mov
the remembered range desynced the probe, which only ever sees props
|
Thanks for the review, Chi.
This is intentional. I defined it this way in case the user wants to switch back to the last custom date range. I think it can be a valid use case. We can change the behavior if we consider it confusing, though. |
intentional: it is the way back to the last custom range
the probe reproduces the trigger's label, so it cannot be popover state
|
The probe and the trigger already shared the label helper. What they did not share was one of its inputs: the trigger kept So after applying a custom range and clicking a preset, the trigger rendered the formatted range and the probe resolved to "Custom". That is your 90px apporx.
Worth knowing when you retest: the probe now measures the wide label whenever a custom range is remembered, so the presets shorten earlier and stay shortened after switching back to a preset. That is the measurement being accurate, not a regression. |
chihsuan
left a comment
There was a problem hiding this comment.
I think it can be a valid use case. We can change the behavior if we consider it confusing, though.
Thanks for clarifying! It may be worth checking with @ederrengifo, as the behavior appears to be different on the demo site. Agreed we can update it in a follow-up if needed.
Worth knowing when you retest: the probe now measures the wide label whenever a custom range is remembered, so the presets shorten earlier and stay shortened after switching back to a preset.
Thanks, tested well now. LGTM! 🚀
…-responsive-labels
There was a problem hiding this comment.
Approving — I went through the probe design, the resolver, and the stories, and this is excellent work. Verified along the way:
- The unconditional double-render probe is the right answer to the oscillation problem, and the rounding directions pair correctly (probe
ceils demand, panelfloors supply — conservative on both sides, so sub-pixel jitter can't flip the mode). resolvePresetLabelMode's tests are a model: the every-tier-reachable set test and the monotonicity property (including the degenerateabbreviated > fullwidths) pin exactly the failure modes a boundary-only suite would miss.- No stray
MOBILE_CONTAINER_WIDTH_THRESHOLDreferences remain;isCompactsurvives only aslabelMode === 'select'. - The abbreviated pills carrying the full label as
aria-label, and the whole group switching together, are both the right calls. - Spot-checked the call-site cleanup: all the removed
containerElementwrappers were indeed passing the panel's own immediate wrapper back.
One finding from testing the storybook stories, worth addressing here since this PR is what makes it reachable:
The custom trigger's font size still hangs on a fixed viewport breakpoint. .date-filters-panel-button gets max(md, 16px) below a @media (min-width: 600px) query (pre-existing, date-range-popover/date-range-filter.scss). On trunk that mismatch was unreachable in the app: the container is always narrower than the viewport, so below 600px viewport the panel was already in select mode and 16px matched the select. With the middle tier, full/abbreviated pills now survive into that zone — e.g. viewport 599px with the panel at ~560px keeps full 13px pills next to a 16px trigger. (In storybook it flips when the canvas iframe crosses 600px — around a 900px outer viewport with the sidebar open.)
Suggestion, in the spirit of the rest of this PR — key it to the panel's measured state instead of the viewport, and the media query goes away:
.date-filters-panel-button {
font-size: var(--wpds-typography-font-size-md);
}
.date-filters-panel.is-compact .date-filters-panel-button {
font-size: max(var(--wpds-typography-font-size-md), 16px);
}The trigger then matches the pills whenever it shares the row with them, and matches the 16px select exactly when the row collapses — which also retires the last width constant this PR's own rationale argues against. Fine as a fast-follow if you'd rather keep this PR frozen.
Screenshots
Viewport 600px — abbreviated pills and the Custom trigger aligned at 13px
Viewport 599px — the media query fires: Custom jumps to 16px beside the 13px pills
Viewport 462px — select mode; both dropdowns at 16px, where the sizes align again
clean; trunk's changes land in the locations report, away from the date controls
a 600px viewport query left a 16px trigger beside 13px pills
|
Thanks, folks. We'll continue polishing the implementation in follow-ups. Thanks for your review 🙇♂️ |
…ment panel Conflicts were the #50964 externals refactor against the video-detail stage/summary-card imports (resolved to the externals convention) and the date-filters panel, taken wholly from trunk — this branch's net change to packages/ui is zero since 'Drop the header date-filter half', and trunk's WOOA7S-1817 content-measurement rework is what that deferral was waiting for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mparison from its widgets (#51082) * Premium Analytics: post detail parity — overflow fixes, timezone, no comparison Applies to the post/email detail page the fixes proven on the video detail page: the breadcrumb/summary overflow bugs (contain: inline-size plus the breadcrumbs-slot shrink shim), aspect-ratio: 1 so the boot shell's img reset cannot squash the featured image, post-views bucket keys parsed as site-local calendar dates (parseSiteDateTime, UTC-12 regression test), and block-size nits. The page's design has no period-over-period comparison: the post-views and email-time-series widgets drop their comparison series, the route normalizes comparison params away, and DateFiltersPanel gains a minimal showComparison prop so the Compare control hides at the existing fixed-bar call site. Moving the panel onto the summary's title row (per the mock) is deferred until the preset measurement rework lands (WOOA7S-1816) — it would collide head-on, the same reason the video-detail PR dropped its filters row. Rebased onto trunk accordingly: nothing here depends on the video-detail branch anymore. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Pass comparison params through the post-detail route Mirror the video-detail review outcome (#50970): the page renders no comparison and its widgets ignore the params, but the breadcrumb's dashboard link reads the URL state back out, so stripping them lost the user's comparison settings on a Dashboard → Post → Dashboard round trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Premium Analytics: align comparison pass-through comments with the route contract * Strip comparison params from injected widget reportParams and drop the parseISO fallback Per review: the page-wide no-comparison invariant now holds by construction — usePostDetailTabs injects comparison-stripped reportParams (via the new omitComparisonReportParams helper) into every layout entry, so comparison-capable widgets like UTM insights and highlights can no longer render deltas from URL state. The URL keeps the comparison params for the breadcrumb round trip. Also remove the parseISO fallback in use-post-views: bucket dates come from format(start, 'yyyy-MM-dd') so parseSiteDateTime cannot fail, and the fallback would silently reintroduce the browser-local day shift; an unparseable bucket now drops the point instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Premium Analytics: restore the video detail date filters and strip comparison from its widgets WOOA7S-1816: the date filter presets were dropped from the video detail page in #50970 because they collided with the preset-measurement rework; now that #50906 has landed, restore them using the same construction as post detail (DateFiltersPanel with showComparison=false, fixed above the scroll container). Also port the comparison-strip construction from #50971: inject omitComparisonReportParams()-stripped reportParams into every layout entry so comparison-capable widgets (highlights) cannot render deltas from URL state, while the URL keeps the params for the breadcrumb round trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Record the crypto-js deprecation in pnpm-lock.yaml The npm registry marked crypto-js@4.2.0 deprecated after this lock was written, so the lock check's fresh resolution now expects the deprecation note. Registry-metadata drift only; no dependency change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Clarify that the video detail comparison strip is defensive All three current video-detail widgets already ignore comparison params in their own query mapping; the injection exists so the page-wide invariant holds by construction (matching post detail) rather than relying on each widget. Adjust the code comment and changelog wording accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Put the date filter presets on the summary title row per the mocks One header row per the design mocks: title left, presets right, vertically centered. The summary grows into free space and absorbs the squeeze down to a 400px floor (title ellipsis); its inline-size containment keeps a long title from wrapping the row. Known rough edge, deferred to #51088: the panel self-measures its root, which in this shrink-to-fit slot always sees its own content width — so the presets keep their full layout and narrow rows degrade poorly. The external-measurement wiring (containerElement / reservedInlineSize) ships there to keep this PR free of shared-component changes beyond the already-reviewed showComparison prop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Put the video detail date filter presets on the summary title row Same composition as post detail: one header row, title left and presets right, 400px summary floor with inline-size containment. The panel's step-down on narrow rows needs external measurement and ships separately (#51088). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop the route-level breadcrumb shim in favor of the component-owned fix Superseded by #51085: the ':has(> nav)' rule targets StatsBreadcrumbs' display:contents trail wrapper (no box for min-inline-size to act on) and the nav rule is covered by the component stylesheet. Tracked upstream as WordPress/gutenberg#81297. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop the video detail route-level breadcrumb shim in favor of #51085 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: retrigger checks — workflow runs were never created for the previous push * ci: retrigger checks — workflow runs were never created for the previous push --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… with a reserved share (#51088) * Premium Analytics: let DateFiltersPanel measure an external container with a reserved share Two props for hosts that place the panel in a shrink-to-fit slot, such as a detail page's header row shared with the summary title: - containerElement: measure the given element instead of the panel's own root. #50906 removed external measurement because every caller then passed its immediate wrapper, making it a no-op; in a shrink-to-fit slot the root's width follows the panel's own content, so self-measurement can neither collapse the layout when narrow nor expand it back when widened. - reservedInlineSize: inline space in the measured container never available to the panel (a title's minimum share). Subtracted before resolving the layout, so the panel steps down while its row-mates still have their minimum. Consumers land in #50971 (post detail) and #51082 (video detail); those branches carry identical copies of this change until it merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Wire the header-row measurement into both detail pages With #50971 / #51082 merged, connect the stages to the new props: each header row gets a ref, and the panel measures it with the summary's reserved share (400px floor + 16px gap + 24px step-down buffer) subtracted — restoring the narrow-row degradation (full → abbreviated → compact) that shipped as a known rough edge in those PRs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Update the wrap comments for the dropped preset select mode Since #51094 the panel's smallest layout is abbreviated pills — there is no compact dropdown to collapse into. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop a stray blank line left by the trunk merge Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Fixes WOOA7S-1816
Part of WOOA7S-1793
What?
The date range presets now shorten their labels before collapsing into a select, and both switches are decided by measuring the active language's own labels rather than comparing the container against a fixed width.
Implements WOOA7S-1816, on the measurements from WOOA7S-1817. Design: Eder's p1785208001025929-slack-C06FSDTSN82, "Responsive format".
Also included, because they surfaced while measuring:
DateFiltersPanelmeasures itself, socontainerElementis gone from 12 call sites.Why?
The panel collapsed its presets into a select below a fixed 600px. That number was picked against English, and English is the shortest case. Measured through CoreText at the pill's real type settings, across the 15 locales that translate all four preset labels:
German is not the worst case, it is seventh. CJK is narrower than English. Russian is the outlier, and the whole row needs 694px there against 470px in English.
Two consequences. A fixed threshold is wrong for almost every language, by up to 225px. And collapsing straight to a select was throwing away a click for no reason: abbreviated labels fit in a fraction of the room, so the select only becomes necessary far below where it used to fire.
Counting characters would not have found this.
Последние 7 днейis 1.45x English by character count but 1.69x by rendered width.How?
PresetLabelMode(full/abbreviated/select) replaces theisCompactboolean.isCompactsurvives only aslabelMode === 'select', for the popover and the compact CSS.PresetRowProberenders the preset row twice off-screen, once per label form,inertand atmax-content, and reports both natural widths. Measured in the DOM rather than computed, because the width depends on the font resolved for the locale's script, the button padding tokens, and the group's borders — reproducing that in JS would be a second implementation of the same layout, free to drift.It renders both forms unconditionally. This is the load-bearing detail: measuring the live row oscillates, since shortening the labels shrinks it, the full labels then look like they fit, and back again.
resolvePresetLabelMode( availableWidth, rowWidths )is a pure function picking the longest form that fits. There is no threshold constant left.WIDE_CALENDAR_CONTAINER_THRESHOLDstays one on purpose: the calendar's content is two month grids, whose width does not follow the locale's strings.The abbreviations are
_x()strings with translator context, not truncations.Last 24Hand7Dare English initials; without context a translator leaves them in English and a Russian reader sees7D.flex-wrap: nowrapon the rolling group, now that shortening happens before overflow. That also retires an artifact: the group's hairline dividers are drawn with> * + *, which knows nothing about row breaks, so a wrapped row's first pill painted a border on top of the group's own frame. Scoped to that group rather than the shared mixin, since the year surface has no abbreviated form to fall back on yet.Self-measurement
Every one of the ten page consumers wrapped the panel in a div and handed it back as
containerElement, so the value was always the panel's own width. The panel now observes its own root. The twoReportParamsFieldcopies also lose adocument.querySelector( '.next-admin-layout__surface' )workaround that existed only to feed the prop, and whose upstream fix (WOOA7S-1008) was cancelled.Net effect on the call sites: 137 lines removed, 32 added.
Testing
Storybook:
cd projects/js-packages/storybook && pnpm run storybook:dev, then Packages / Premium Analytics / UI / DateFiltersPanel.TranslatedRussian,TranslatedDutchandTranslatedGermaninstall real WordPress.org language packs throughsetLocaleData, so labels come from the actual__()path. Each renders the bar at 960 / 782 / 600 / 360 / 280px plus a resizable container.Last 24H 7D 30D 12M. On trunk it wraps to two rows.DashboardFilters) at 600px keeps full labels. Same width, different tier, because the boundary is measured per locale.Also worth a pass in wp-admin on the dashboard, a report page, and post detail, since all three lost their wrapper ref: narrow the browser and confirm the presets shorten and then collapse.
Automated:
pnpm run testinprojects/packages/premium-analytics(199 suites, 1395 tests), pluspnpm run typecheck.Follow-ups
Last 24Hand friends, non-English users see English initials in the middle tier. Tracked on WOOA7S-1817.getBoundingClientRect()returns 0, the probe bails, and the panel stays infullfor the whole suite. The decision function and the label rendering are covered; the measurement is verified visually. Worth knowing before trusting a green run on this component.DateYearFilterstill takescontainerElement. Its panel lands in WOOA7S-1815, which should give it the same treatment.flex-wrap: wrapbecause it has no abbreviated form. Revisit with WOOA7S-1815.All timeorphans the existingAll-timetranslation over a hyphen, andCustomneeds_x()context — the French pack returns "Lien" for the bare msgid.