Repository navigation
Premium Analytics: fit Highlights at the minimum tile height - #51052
Conversation
|
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! |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
…components Three components already dropped chrome at the same hardcoded 140px, and this branch added a fourth. Hoist it to $widget-short-max-block-size in the toolkit's shared partial and have the SCSS consumers read it; the widget stylesheet is plain CSS, so it keeps the literal with a pointer to the source. Also switch metric-tile-grid's container queries to logical properties, refresh the Highlights docblock and Storybook description now that the year selector is gone, add a wide-and-short dashboard story for the layout this branch changes, and reword the changelog to cover the year-selector removal.
There was a problem hiding this comment.
LGTM — reviewed and verified on a local docker site (dashboard + both detail pages).
Verified locally:
- Height-2 default: unchanged apart from the removed year selector;
See reportand roomy spacing intact. - Wide × height-1: all four values fit on one row, footer hidden, tighter tiles — matches the PR screenshots.
- Narrow × height-1:
See reportstays, rows scroll — identical to trunk, as intended. - Post/video detail highlights (the other
MetricTileGridconsumers in wide height-1 cells, which the shared$widget-short-max-block-sizethreshold now also covers): the tightened tiles render fine there too. - Nice touch converting the container queries to logical properties while extracting the shared thresholds.
3x1
| Before | After |
|---|---|
![]() |
![]() |
3x2
| Before | After |
|---|---|
![]() |
![]() |
Pre-existing bug this PR is well placed to fix (one line, not blocking): with the default Insights layout, the Highlights widget renders "Select at least one metric to display." while the Metrics dropdown shows every metric checked. The default layout instance is created without attributes (get_dashboard_default_widget_instance( 'default-annual-highlights-widget-instance', 'jpa/annual-highlights', 0, 2, 2 ) in src/dashboard-layout.php — no $attributes), and render.tsx passes attributes.metrics straight through with no fallback, so a missing attribute becomes an empty Set and zero tiles — while the inline control renders the unset state as all-checked, so the UI contradicts itself. Suggest metrics={ attributes.metrics ?? DEFAULT_HIGHLIGHT_METRICS } at the AnnualHighlightsReport call site (the constant already exists in widget.ts), either here since this PR reworks the widget anyway, or as a quick follow-up — happy to open it if you prefer. Repro note: toggling any metric off/on writes the attribute and masks the bug on that site afterwards, which is probably why it went unnoticed.
|
Thanks for the review! @dognose24 Good catch — folded it in.
|




Proposed changes
Update
Highlightswidget to match design:Annual Highlightsyear selector.See reportfooter in wide cells when the widget body is under 140px tall.These changes let all four values fit at the minimum widget height. Taller widgets keep the
See reportfooter and the roomier tile spacing.The footer has to go in those cells rather than shrink: the body is 86px, the footer takes 31px of it, and the metric tile still needs 82px after the tighter spacing. Nothing fits both. The design prototype's height-1 Highlights carries no report link either.
Year selection moves to the per-section control in #50930 — the prototype has no in-widget year control at any size. Pointing these numbers at that selection is a follow-up once #50930 lands; until then the widget shows the most recent year. Narrow cells keep the report link and stay scrollable: four compact rows cannot fit in the available height whether or not the link is there, so dropping it would buy nothing. The default Insights layout uses height 2, so neither limitation affects the default experience.
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
cd projects/js-packages/storybook && pnpm run storybook:dev.See report:http://localhost:50240/iframe.html?id=packages-premium-analytics-widgets-annualhighlights--widget-dashboard-with-widget&viewMode=story&args=widgetWidth%3A4%3BwidgetHeight%3A1%3BrowHeight%3A200See reportstays, rows scroll: same URL withargs=widgetWidth%3A1%3BwidgetHeight%3A1%3BrowHeight%3A200Before:

After:
