Repository navigation
Premium Analytics: align the Ads tab with the design - #52714
Conversation
Title the tab "Ads performance" from both registrants of the section, the WordAds module and jetpack-mu-wpcom, as the other tabs carry a title. Reword the three WordAds widget tips to the design copy: the earnings list explains whole billing months, the balance explains lifetime totals and the $100 payout threshold, and the summary links to the WordAds support page. On the Earnings report, the Period column is bold as the row's label, and the Ads Served note above the table is gone: no other report has one, and the support link now explains the figure. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dognose24
added
Enhancement
dognose24
added
Enhancement
|
@chihsuan the Ads tab design alignment from WOOA7S-2160: tab title, the three widget tips, and the Earnings report table. Screenshots to follow. |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Feature] Ad
[mu wpcom Feature] Premium Analytics
[Package] Jetpack mu wpcom
|
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. Wpcomsh plugin:
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 4 files.
|
| /> | ||
| ) : ( | ||
| <> | ||
| { showAdsServed && ( |
There was a problem hiding this comment.
Why do you remove this flag?
btw, having hasAdsServed looks excessive to me
/**
* Whether a tab's rows carry an Ads Served count.
*
* Only the WordAds bucket records pageviews; sponsored and adjustment rows are
* amounts alone.
*
* @param tab - The active Earnings report tab.
* @return Whether to show the Ads Served column and its note.
*/
export function hasAdsServed( tab: EarningsReportTabId ): boolean {
return tab === 'wordads';
}You could define the const...
const showAdsServed = tab === 'wordads';Even more, I'd suggest not creating a const and simply using the tab === 'wordads'.
There was a problem hiding this comment.
The flag is still there: it gates the Ads Served column and the CSV column a few lines up. What went was the note it also guarded, since the design has none and no other report carries one; the Ads summary tip's Learn more link now explains the figure. As it stands after 26ce42a:
const showAdsServed = tab === 'wordads';
const fields = useMemo( () => {
const all = getWordAdsHistoryFields();
return showAdsServed ? all : all.filter( field => field.id !== 'pageviews' );
}, [ showAdsServed ] );
const csvColumns = useMemo( () => [
/* Period, Earnings */
...( showAdsServed ? [ { label: __( 'Ads Served' ), getValue: row => row.pageviews } ] : [] ),
/* Status */
], [ showAdsServed ] );Agreed on the helper, done in 26ce42a: it is now const showAdsServed = tab === 'wordads' in the page, and hasAdsServed is gone from the config, its test and the CSV test's mock. Kept the const because two useMemo deps read it.
The design draws the period as the row's title: bold, in the content colour. A <strong> only got the weight; the table's cells stay in the weak colour. DataViews' titleField gives both, so the view names it and the field renders plain text again. ReportRecordsTable seeded every field into view.fields, which drew a title field twice. It now leaves the primary fields out of that list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A one-line helper for `tab === 'wordads'` was more indirection than the check deserved. The page reads it inline; the helper, its test and the CSV test's mock of it are gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks @dognose24 Looks good and tested well.
Just a thought: I think we should check with @ederrengifo to confirm whether we really want to use the badge as a link in the widget footer. To me, the badge doesn’t look obviously clickable and I believe we're not using similar pattern in other places.
|
Thanks! Agreed on the badge, and that question is already parked as WOOA7S-2194 with the alternatives I tried (plain link plus a count badge, and |
github-actions
Bot
added
[Status] UI Changes
3 participants
Fixes WOOA7S-2160
Proposed changes
The remaining differences between the Ads tab and Eder's design, after the status badges (#52565) and the adjustments badge (#52526) landed. Each one is small; together they finish the card.
Tab title. The Ads section registered no
title, so its heading fell back to the tab label, Ads. It is now Ads performance, as in the design, set by both registrants of the section since #52455: the WordAds module for the Jetpack plugin path andjetpack-mu-wpcomfor WordPress.com. The other tabs already carry a title this way (Site traffic, Site insights).Widget tips, reworded to the design copy:
The design's balance tip has no payout threshold; the last sentence keeps it, because classic Stats states it in its Totals tip and it is the one thing a publisher checks when a payment has not arrived.
Earnings report. Period is the view's
titleField, so DataViews draws it as the row's title: bold, in the content colour rather than the table's weak grey, as in the design.ReportRecordsTableseeded every field intoview.fields, which would draw a title field twice; it now leaves the primary fields out of that list, with a test. The Ads Served note above the table is gone: no other report carries a note, the design has none, and the support link on the Ads summary tip now explains the figure. The report keeps its Earnings history name (the design's "WordAds earnings" predates the Sponsored and Adjustments tabs) and its August 2026 period format, which every report shares; both are noted on the card.Not here: the Metrics dropdown on All-time balance, removed in #52700.
Local Docker site with mocked WordAds data. The before shots predate #52700, so the Metrics dropdown still shows there.
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
On a site with WordAds, or the local Docker site with the
zz-tmp-pa-wordads-mock.phpmu-plugin:dataviews-title-fieldstyling); there is no note above the table.jp test js packages/premium-analytics,jp test php packages/jetpack-mu-wpcom, andjp docker phpunit jetpack -- --filter WordAds_Premium_Analytics_Testpass.🤖 Generated with Claude Code