Repository navigation
Premium Analytics: mark an active date comparison with vs and its dates - #51820
Conversation
a link carrying only a preset compared nothing but painted the control active
the trigger's abbreviation says which preset, not which window
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
kangzj
left a comment
There was a problem hiding this comment.
Works well for me 👍 The vs and the new tooltip both do what the description says, and I could reproduce the gate bug on the merge base and watch it go away on this branch, so approving.
One thing that has nowhere to hang off the diff: the WithCustomLabel story still describes label as the thing that "becomes the trigger's tooltip". That's the date range now whenever a comparison is active, and the story defaults to enabled, so autodocs ends up showing the new tooltip under a description of the old one. Small tweak tho. Left one more note inline on the vs typography.
Testing
Storybook on this branch (Packages/Premium Analytics/UI/DateComparisonDropdown and DateFiltersPanel), plus the merge base for the before/after. I couldn't get onto the actual wp-admin dashboard in my env, so the two gates went through the PR's own new tests run against the merge-base code instead of a real deep link.
| What I checked | What happened | Status |
|---|---|---|
| Additive state, no comparison | Compare +, no vs, no tooltip on hover, button named "Compare" |
✅ |
| Active state | vs + Prev. period, and the vs goes the moment the comparison does |
✅ |
| Trigger tooltip names the compared window | "July 3 – August 2, 2026"; picking "Same period in 2025" moves it to "August 2 – September 1, 2025" | ✅ |
Menu keeps its own name (the new menuProps) |
menu aria-label stays "Compare to" while the toggle's tooltip is the dates |
✅ |
| Tooltip follows the primary range | Primary Aug 18 – 30, tooltip August 6 – 18, 2026, the window right before it |
✅ |
Deep link carrying compare_preset with no window |
Merge base hands back previous-period, this branch hands back undefined |
✅ |
Widget-owned control with a saved compare_preset and no window |
Merge base renders Prev. period, this branch renders Compare |
✅ |
| Keyboard: ArrowDown on the trigger, after the new wrapper div | Menu opens | ✅ |
| Row layout in German and Russian at 960 / 782 / 600 / 360 / 280px | vs sits between the two triggers, nothing wraps or overflows |
✅ |
jp test js packages/premium-analytics |
2981 passed, plus the LA / Tokyo timezone pass | ✅ |
| typecheck, eslint, stylelint on the changed files | Clean | ✅ |
vs typography against the rest of the row |
Comes out 16px / #000 next to controls pinned to 13px / #1e1e1e — note inline | |
| The dashboard in wp-admin | Couldn't get onto it in my env, so it went through Storybook and the tests above instead |
Evidence
DateFiltersPanel → CustomRangeWithComparison, hovering the comparison trigger.
| Before | After |
|---|---|
![]() |
![]() |
Thanks @retrofox!
| // Reads with the trigger beside it rather than as a control of its own, so | ||
| // it takes the row's own type and only refuses to wrap. | ||
| &__prefix { | ||
| white-space: nowrap; | ||
| } |
There was a problem hiding this comment.
The toggle right beside this pins itself to --wpds-typography-font-size-md, but the prefix just inherits, so the two only line up when the host body size happens to match the token. In Storybook the vs lands at 16px / #000 next to a row of 13px / #1e1e1e controls - noticeably bigger and blacker than the trigger it belongs to. Probably fine in wp-admin where body is 13px anyway, but could it take the same token (plus the max(…, 16px) bump the toggle has under 600px)? I might be missing sth here.


Proposed changes
vsprefix to its left, as the design asks:vs [Prev. period ▾].June 1 – 30, 2026) instead of repeating the control's own name. The trigger's text is an abbreviation of the preset, and since#51707removed the section header subtitle there was nowhere left showing the dates.comparisonPresetIdis now gated onhasComparisonEnabled, the same condition the widgets fetch on, in both places that feed the panel:useReportDateFiltersand the widget-ownedReportParamsField. A link carryingcompare_presetwith nocomp/compare_from/compare_toused to read "Prev. period" while nothing was compared and no delta badges rendered. Raised by @kangzj on#51779; it predates that PR and applies to every preset.The tooltip reads the selected option's own resolved range rather than a new prop threaded down from the URL. That range already follows the same source the rest of the row's labels do (the draft while the picker is open, the applied window while it is closed), so the tooltip cannot disagree with the label beside it.
vslives insideDateComparisonDropdownrather than in the panel: the panel renders the control twice, once in the row and once in the measuring probe, and only markup the component owns is guaranteed to be identical in both.Still open
The active trigger keeps today's abbreviation (
Prev. period). Whether it should instead carry the full dynamic label (Previous 30 days) withvsoutside it is a design question the prototype does not answer, since it only shows the inactive state. Nothing here forecloses either choice.Related product discussion/links
#51771(off by default) and#51779(options derived from the range).Does this pull request change what data or activity we track or use?
No.
Testing instructions
jetpack build --deps packages/premium-analytics.vs.vsappears to the left of the trigger, and the trigger names the preset as before.vsgoes away with the comparison.comp,compare_fromandcompare_tofrom it, keepingcompare_preset. Reload. The control readsComparewith novs, matching the widgets, which show no delta badges. On trunk, it reads "Prev. period" instead.reportParamscarrycompare_presetalone showsCompare.Screen.Recording.2026-09-01.at.3.12.13.PM.mov
Storybook:
Packages/Premium Analytics/UI/DateComparisonDropdown→Default(active) andNoComparison(additive).