Premium Analytics: give the post detail email tabs an email header identity - #51260
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 2 files.
|
|
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! |
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this! @dognose24 Overall, this is looking good.
Just left a few minor comments.
I also noticed one thing we could consider improving in a follow-up. Currently when page is loading, it show a default description and placeholder image. I think we can considering showing loading spinner or skeleton instead (after skeleton PR is merged).
Screen.Recording.2026-08-14.at.10.37.30.AM.mov
| performance window stay the post's. */ } | ||
| <PostSummaryCard | ||
| summary={ summary } | ||
| variant={ activeTab === 'post-traffic' ? 'post' : 'email' } |
There was a problem hiding this comment.
nit: Could we key this off the email tab list instead of excluding post-traffic?
We can exporting EMAIL_TAB_IDS and inverting the check makes the default fail closed:
variant={ EMAIL_TAB_IDS.includes( activeTab ) ? 'email' : 'post' }There was a problem hiding this comment.
Done in 12d0035 — exported EMAIL_TAB_IDS from the tabs config and keyed the variant off it, so a future tab defaults to the post identity unless added to the list.
| // weak interactive tokens are the only weak brand pair in the set; the tile | ||
| // is not interactive, but the mapping keeps it on theme tokens. | ||
| .emailTile { | ||
| background: var(--wpds-color-background-interactive-brand-weak-active); |
There was a problem hiding this comment.
Worth using --wpds-color-background-surface-brand instead here?
The comment above says the weak interactive tokens are the only weak brand pair, but --wpds-color-background-surface-brand (#ecf0fa) is the non-interactive brand surface.
Screen.Recording.2026-08-14.at.10.50.07.AM.mov
There was a problem hiding this comment.
You are right — --wpds-color-background-surface-brand is the non-interactive brand surface, my comment had it backwards. Switched the tile background to it (and corrected the comment) in 12d0035.
…brand surface token Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the review! Both nits addressed in 12d0035. Agreed on the loading treatment — once the skeleton PR lands, the header placeholder is a good candidate to adopt it; noted as a follow-up. |
Proposed changes
Per-tab header identity for the post detail page, from WOOA7S-1762 and the updated design mocks:
The sent date reuses the publish date: WordPress.com sends the newsletter when the post publishes, and the email stats API exposes no separate send timestamp (verified against
stats/opens/emails/{id}/rate,stats/emails/summary— whosedateequals the post date — and the timeline endpoint). If a real send timestamp ever lands server-side, only the value feeding the sentence changes.Out of scope (still on WOOA7S-1762): the "Performance: All time" subtitle wording, which depends on the All time preset work under WOOA7S-1793.
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
packages/premium-analyticsand open the Premium Analytics dashboard.pnpm run test routes/post-detailpasses.🤖 Generated with Claude Code
Screenshots
Email opens tab header.
Email clicks tab header.