Premium Analytics: post detail parity — overflow fixes, inline date presets, no comparison - #50971
Conversation
|
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 🤖 🔴 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! |
7b10fb9 to
ed03ae3
Compare
…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>
ed03ae3 to
9ee2ef7
Compare
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>
…ost-detail-parity # Conflicts: # projects/packages/premium-analytics/packages/ui/src/date-filters-panel/date-filters-panel.tsx # projects/packages/premium-analytics/routes/post-detail/package.json # projects/packages/premium-analytics/routes/post-detail/stage.tsx
chihsuan
left a comment
There was a problem hiding this comment.
Thanks @dognose24
I noticed UTM and highlights are still rendering comparison data. I wonder if it's possible to preserve the URL for the breadcrumb round trip, but remove comp/compare_* from the reportParams injected into every widget in this page’s layout? That would make the page-wide invariant hold by construction.
| // as UTC midnight and read as the previous day on negative-offset sites). | ||
| return buckets.map( bucket => ( { | ||
| date: localTZDate( bucket.date ), | ||
| date: parseSiteDateTime( bucket.date ) ?? parseISO( bucket.date ), |
There was a problem hiding this comment.
Could we remove this fallback instead of allowing it to produce the wrong instant?
bucket.date is generated with format(start, 'yyyy-MM-dd'), so parseSiteDateTime() should always succeed here. If it ever does not, parseISO(bucket.date) parses browser-local midnight rather than site-local midnight, reintroducing the day-shift bug this change fixes.
There was a problem hiding this comment.
Fixed in 10fc2e2 — dropped the fallback. bucket.date comes from format( start, 'yyyy-MM-dd' ) so parseSiteDateTime() can't fail in practice; if it ever does, the point is now dropped (flatMap) rather than re-anchored to a browser-local instant that would reintroduce the day shift.
…ost-detail-parity # Conflicts: # projects/packages/premium-analytics/routes/post-detail/stage.test.tsx
…e 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>
|
Thanks @chihsuan! Adopted your suggestion in 10fc2e2 — the invariant now holds by construction.
The video detail page (#50970) uses the same widgets-ignore approach — I'll port this construction there as a follow-up. |
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>
chihsuan
left a comment
There was a problem hiding this comment.
Thanks @dognose24 Looks good. 🚢
98f4d81 to
72de229
Compare
|
A note on the current shape of this branch since your last review: it is the state you reviewed, plus your two notes addressed (comparison stripped from the injected reportParams via omitComparisonReportParams(), parseISO fallback dropped), a one-line pnpm-lock refresh for the crypto-js deprecation flag, and one commit (b67f72b) that puts the date filter presets on the summary's title row per the design mocks — title left, presets right, 400px summary floor. No shared-component changes beyond the already-reviewed showComparison prop: the panel keeps measuring itself, which in this shrink-to-fit slot always sees its own content width — so the presets stay in their full layout and narrow rows degrade poorly. That is a known, accepted rough edge here; the external-measurement wiring (containerElement / reservedInlineSize) ships as a follow-up in #51088. The breadcrumb overflow fix lives in #51085. |
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>
a91debd to
b67f72b
Compare
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>
…ost-detail-parity
…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>

Part of WOOA7S-1785 · Based on trunk (no longer stacked on #50970 — nothing here depends on it after the rescope below)
Why
Applies to the post/email detail page everything proven out on the video detail page (#50970): the overflow bugs found by live debugging, the timezone day-shift, and the mock's inline date presets without a comparison control.
Proposed changes
contain: inline-size(long unbroken titles no longer force page-level horizontal scroll; the breadcrumb half of that fix is component-owned in Premium Analytics: fix the breadcrumb overflow behind the display:contents trail #51085, tracked upstream as admin-ui: Page header breadcrumbs slot lets a nowrap crumb's min-content widen the page, defeating the crumb's own ellipsis WordPress/gutenberg#81297);aspect-ratio: 1so the boot shell'simgreset can't squash the featured image;widgets/post-viewsbucket keys parsed as site-local calendar dates (parseSiteDateTime, UTC-12 regression test) fixing the one-day shift on negative-offset sites;height→block-sizenit.post-viewsandemail-time-seriesdrop their comparison series, andDateFiltersPanelgains a minimalshowComparisonprop so the Compare control hides at the existing fixed-bar call site. Comparison params stay in the URL untouched (the widgets ignore them) so the breadcrumb's dashboard link carries the user's comparison settings back out — mirroring the review outcome on Premium Analytics: align the video detail composition with the design mocks #50970.contain: inline-sizekeeps a long title from wrapping the row. Known rough edge, deferred to Premium Analytics: let DateFiltersPanel measure an external container with a reserved share #51088: the panel still measures its own 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 until the external-measurement wiring lands there, for both detail pages at once.Testing
pnpm testinprojects/packages/premium-analytics— 1622 tests green, including the new UTC-12 bucket regression test and the no-comparison assertions.Screenshots
Date filter on desktop
Wrapped date filter on mobile