Skip to content

Commit 867ec4d

Browse files
authored
jingram/cpm-stats-on-ntp-with-scroll (#2073)
* Update animation test cases * Add messaging subscription to scroll to protections report * Track last seen blocked values * Fix initial tracker count TickPill display Handle undefined trackingStatus when sites are first logged. The component now provides a safe default and reactively updates when tracking status data arrives, fixing the issue where "no trackers blocked" was shown initially even when trackers were actually blocked. * Update tracking status Updated the component to use reactive computed signals. Changes: 1. `trackingStatus` — computed signal tracking activity.value.trackingStatus[id] 2. `totalTrackersBlocked` — computed signal derived from trackingStatus 3. `totalTrackersPillText` — computed signal that updates when the count changes 4. `cookiePopUpBlocked` — kept as a computed signal (removed the .value that read it once) * Another attempt at updating tracking status 1. Added `useSignalEffect`: Tracks when computed signals (totalTrackersPillText, totalTrackersBlocked, cookiePopUpBlocked) change. 2. Used a ref to track previous values: prevValuesRef stores the last values to avoid unnecessary re-renders. 3. State update on change: When computed values change, update state via setRenderKey to trigger a re-render. 4. Conditional updates: Only update state when values actually change, reducing unnecessary re-renders. How it works: • useSignalEffect runs whenever the computed signals change. • It compares current values with previous values stored in the ref. • If values changed, it updates the ref and calls setRenderKey to force a re-render. • The re-render causes TickPill to display the updated values. * One more attempt 1. Created `activityData` computed: Tracks activity.value directly so when normalizeData creates a new object (line 228 in NormalizeDataProvider), this computed re-evaluates. 2. Updated all computed signals: They now depend on activityData.value instead of activity.value directly, ensuring they re-evaluate when activityData changes. 3. Added `useSignalEffect`: Tracks activityData changes and forces a re-render via state update when activity.value changes. 4. Access computed values during render: Accessing .value during render (not just in JSX) ensures Preact Signals tracks them for reactivity. How it works: • When trackingStatus data arrives via activity_onDataUpdate or activity_onDataPatch, normalizeData runs • normalizeData creates a new activity.value object (new reference) • activityData computed detects the change and re-evaluates • All dependent computed signals (trackingStatus, totalTrackersBlocked, etc.) re-evaluate • useSignalEffect runs and calls setRenderKey to force a re-render • Component re-renders with updated TickPill values * Address ship review UI comments * Address linter issues * Address linter issues * Minor CSS tweak for tick pill (chip) * Revert Activity.js changes These changes are not needed until we decide if we will keep the blocked counts aligned with native * Revert hook edits unlrelated to the scroll feature Part 2 * Address Cursor review Bug: IntersectionObserver not set up due to ref timing The useEffect that sets up the IntersectionObserver has [elementRef] as its dependency, but ref objects maintain a stable identity - only elementRef.current changes after the DOM renders. When the component first mounts, elementRef.current is null, causing the effect to set isInViewport(true) and return without creating the observer. Since elementRef (the object itself) never changes, the effect won't re-run after the DOM assigns ref.current. This defeats the viewport detection feature, causing animations to always start immediately instead of waiting for the element to be visible. * Address Stale lastSeenValueRef causes visual regression on re-entry When the targetValue changes while the element is out of viewport, the else-if branch (lines 96-100) correctly snaps the display to the new target but fails to clear lastSeenValueRef. This ref retains a stale mid-animation value from when the element exited viewport. When the element later re-enters viewport, the condition at line 88 evaluates true (stale value differs from new target), causing the animation to start from the old saved value instead of the current displayed value. This produces a visible backward jump in the counter before it animates forward. * Convert "NEW" SVG to component to accept translations * Address linter errors More linter issues * Uppercase new badge and edit tooltip text * Add translations from Smartling
1 parent 6f675d9 commit 867ec4d

File tree

25 files changed

+455
-644
lines changed

25 files changed

+455
-644
lines changed

special-pages/pages/new-tab/app/components/Icons.js

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -602,35 +602,6 @@ export function CloseSmallIcon(props) {
602602
);
603603
}
604604

605-
/**
606-
* @param {import('preact').JSX.SVGAttributes<SVGSVGElement>} props
607-
*/
608-
export function NewBadgeIcon(props) {
609-
return (
610-
<svg xmlns="http://www.w3.org/2000/svg" width="42" height="16" viewBox="0 0 42 16" fill="none" {...props}>
611-
<path
612-
d="M0 3.99792C0 1.78879 1.79086 -0.0020752 4 -0.0020752H38C40.2091 -0.0020752 42 1.78879 42 3.99792V11.9979C42 14.2071 40.2091 15.9979 38 15.9979H4C1.79086 15.9979 0 14.2071 0 11.9979V3.99792Z"
613-
fill="#F9BE1A"
614-
/>
615-
<path
616-
d="M13.0913 9.1073H13.1812V3.94617H14.8032V12.0497H13.3999L9.64893 6.86707H9.55908V12.0497H7.93604V3.94617H9.35107L13.0913 9.1073Z"
617-
fill="black"
618-
fill-opacity="0.96"
619-
/>
620-
<path
621-
d="M22.144 5.3446H18.4722V7.29871H21.936V8.60144H18.4722V10.6512H22.144V12.0497H16.7759V3.94617H22.144V5.3446Z"
622-
fill="black"
623-
fill-opacity="0.96"
624-
/>
625-
<path
626-
d="M26.4663 9.73621H26.5562L28.0337 3.94617H29.4653L30.9702 9.73621H31.0601L32.312 3.94617H34.064L31.9136 12.0497H30.3247L28.7915 6.59167H28.7017L27.1851 12.0497H25.5854L23.4399 3.94617H25.2036L26.4663 9.73621Z"
627-
fill="black"
628-
fill-opacity="0.96"
629-
/>
630-
</svg>
631-
);
632-
}
633-
634605
/**
635606
* @param {import('preact').JSX.SVGAttributes<SVGSVGElement>} props
636607
*/
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { h } from 'preact';
2+
import styles from './NewBadge.module.css';
3+
4+
/**
5+
* Badge component that displays text in a yellow rounded rectangle.
6+
*
7+
* @param {object} props
8+
* @param {string} props.text - The text to display in the badge
9+
* @param {import('preact').ComponentProps<'span'>} [props.rest] - Additional HTML attributes
10+
*/
11+
export function NewBadge({ text, ...rest }) {
12+
return (
13+
<span class={styles.badge} {...rest}>
14+
{text?.toUpperCase() || 'NEW'}
15+
</span>
16+
);
17+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
.badge {
2+
display: inline-flex;
3+
align-items: center;
4+
justify-content: center;
5+
width: fit-content;
6+
height: 16px;
7+
padding: 0 8px;
8+
background-color: #F9BE1A;
9+
border-radius: 4px;
10+
font-family: var(--theme-font-family, system-ui);
11+
font-weight: 700;
12+
font-size: 11px;
13+
color: var(--color-black-at-96);
14+
letter-spacing: -0.015em;
15+
flex-shrink: 0;
16+
}

special-pages/pages/new-tab/app/components/TickPill/TickPill.module.css

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
.tickPill {
22
display: flex;
33
align-items: center;
4-
gap: 6px;
5-
padding: 8px 10px;
6-
border-radius: 100px;
4+
gap: 4px;
5+
padding: 8px 8px;
6+
border-radius: 1000px;
77
background-color: var(--color-white-at-3);
88
border: 1px solid var(--color-white-at-12);
99
height: 20px;
@@ -32,8 +32,8 @@
3232

3333
/* Light mode styles */
3434
[data-theme="light"] .tickPill {
35-
background-color: var(--color-black-at-4);
36-
border: 1px solid var(--color-black-at-12);
35+
background-color: var(--color-black-at-1);
36+
border: 1px solid var(--color-black-at-9);
3737
}
3838

3939
[data-theme="light"] .text {

special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.module.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
height: 16px;
4040
display: inline-block;
4141
vertical-align: middle;
42+
margin-top: 2px;
4243
}
4344

4445
.widgetExpander {

special-pages/pages/new-tab/app/privacy-stats/strings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"note": "The heading indicating multiple cookie pop-ups were handled by the CPM"
5757
},
5858
"stats_protectionsReportInfo": {
59-
"title": "Displays tracking attempts blocked in the last 7 days, and the number of cookie pop-ups blocked since you started using the browser. <span>You can reset these stats using the Fire Button.</span>",
59+
"title": "Displays tracking attempts blocked in the last 7 days, and the number of cookie pop-ups blocked since you started using the browser.",
6060
"note": "Text explaining how to reset the protections stats"
6161
},
6262
"stats_feedCountBlockedSingular": {

special-pages/pages/new-tab/app/protections/components/Protections.module.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
}
4545

4646
.block {
47-
margin-top: 32px;
47+
margin-top: 16px;
4848
}
4949

5050
/* @todo legacyProtections: Remove once all platforms support the new UI */

special-pages/pages/new-tab/app/protections/components/ProtectionsHeading.js

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import { useTypedTranslationWith } from '../../types.js';
1+
import { useTypedTranslationWith, useMessaging } from '../../types.js';
22
import styles from '../../privacy-stats/components/PrivacyStats.module.css';
33
import { ShowHideButtonCircle } from '../../components/ShowHideButton.jsx';
44
import cn from 'classnames';
55
import { h } from 'preact';
6-
import { InfoIcon, NewBadgeIcon } from '../../components/Icons.js';
6+
import { InfoIcon } from '../../components/Icons.js';
7+
import { NewBadge } from '../../components/NewBadge.js';
78
import { Tooltip } from '../../components/Tooltip/Tooltip.js';
89
import { useAnimatedCount } from '../utils/useAnimatedCount.js';
10+
import { useRef, useEffect } from 'preact/hooks';
911

1012
/**
1113
* @import enStrings from "../strings.json"
@@ -29,6 +31,9 @@ export function ProtectionsHeading({
2931
totalCookiePopUpsBlockedSignal,
3032
}) {
3133
const { t } = useTypedTranslationWith(/** @type {Strings} */ ({}));
34+
const ntp = useMessaging();
35+
const headingRef = useRef(/** @type {HTMLDivElement|null} */ (null));
36+
const counterContainerRef = useRef(/** @type {HTMLDivElement|null} */ (null));
3237
const totalTrackersBlocked = blockedCountSignal.value;
3338
const totalCookiePopUpsBlockedValue = totalCookiePopUpsBlockedSignal.value;
3439

@@ -38,9 +43,16 @@ export function ProtectionsHeading({
3843
? Math.max(0, Math.floor(totalCookiePopUpsBlockedValue))
3944
: 0;
4045

41-
// Animate both tracker count and cookie pop-ups count
42-
const animatedTrackersBlocked = useAnimatedCount(totalTrackersBlocked);
43-
const animatedCookiePopUpsBlocked = useAnimatedCount(totalCookiePopUpsBlocked);
46+
// Animate both tracker count and cookie pop-ups count when counterContainer is in viewport
47+
const animatedTrackersBlocked = useAnimatedCount(totalTrackersBlocked, counterContainerRef);
48+
const animatedCookiePopUpsBlocked = useAnimatedCount(totalCookiePopUpsBlocked, counterContainerRef);
49+
50+
// Subscribe to scroll message
51+
useEffect(() => {
52+
return ntp.messaging.subscribe('protections_scroll', () => {
53+
headingRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start' });
54+
});
55+
}, [ntp]);
4456

4557
// Native does not tell the FE if cookie pop up protection is enabled but
4658
// we can derive this from the value of `totalCookiePopUpsBlocked` in the
@@ -54,7 +66,7 @@ export function ProtectionsHeading({
5466
animatedCookiePopUpsBlocked === 1 ? t('stats_totalCookiePopUpsBlockedSingular') : t('stats_totalCookiePopUpsBlockedPlural');
5567

5668
return (
57-
<div class={styles.heading} data-testid="ProtectionsHeading">
69+
<div class={styles.heading} data-testid="ProtectionsHeading" ref={headingRef}>
5870
<div class={cn(styles.control, animatedTrackersBlocked === 0 && styles.noTrackers)}>
5971
<span class={styles.headingIcon}>
6072
<img src={'./icons/Shield-Check-Color-16.svg'} alt="Privacy Shield" />
@@ -79,7 +91,7 @@ export function ProtectionsHeading({
7991
</span>
8092
)}
8193
</div>
82-
<div class={styles.counterContainer}>
94+
<div class={styles.counterContainer} ref={counterContainerRef}>
8395
{/* Total Trackers Blocked */}
8496
<div class={styles.counter}>
8597
{animatedTrackersBlocked === 0 && <h3 class={styles.noRecentTitle}>{t('protections_noRecent')}</h3>}
@@ -99,9 +111,9 @@ export function ProtectionsHeading({
99111
<h3 class={styles.title}>
100112
{animatedCookiePopUpsBlocked} <span>{cookiePopUpsBlockedHeading}</span>
101113
</h3>
102-
{/* @todo `NewBadgeIcon` will be manually removed in
114+
{/* @todo `NewBadge` will be manually removed in
103115
a future iteration */}
104-
<NewBadgeIcon />
116+
<NewBadge text={t('protections_newBadge')} />
105117
</div>
106118
)}
107119
</div>

special-pages/pages/new-tab/app/protections/integrations-tests/protections.page.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,24 @@ export class ProtectionsPage {
153153
await tooltipTrigger.press('Space');
154154
await expect(tooltip).toBeVisible();
155155
}
156+
157+
/**
158+
* Triggers the scroll to protections heading via subscription message
159+
*/
160+
async scrollsToProtectionsHeading() {
161+
const heading = this.context().getByTestId('ProtectionsHeading');
162+
await expect(heading).toBeVisible();
163+
164+
// Scroll away from the element first
165+
await this.page.evaluate(() => window.scrollTo(0, document.body.scrollHeight));
166+
167+
// Trigger the scroll message
168+
await this.ntp.mocks.simulateSubscriptionMessage(named.subscription('protections_scroll'), {});
169+
170+
// Wait for smooth scroll animation to complete
171+
await this.page.waitForTimeout(500);
172+
173+
// Verify the heading is now in viewport
174+
await expect(heading).toBeInViewport();
175+
}
156176
}

special-pages/pages/new-tab/app/protections/integrations-tests/protections.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,14 @@ test.describe('protections report', () => {
9898
await protections.ready();
9999
await protections.hasInfoTooltip();
100100
});
101+
102+
test('scrolls to protections heading via subscription message', async ({ page }, workerInfo) => {
103+
const ntp = NewtabPage.create(page, workerInfo);
104+
await ntp.reducedMotion();
105+
await ntp.openPage({ additional: { 'protections.feed': 'activity' } });
106+
107+
const protections = new ProtectionsPage(ntp);
108+
await protections.ready();
109+
await protections.scrollsToProtectionsHeading();
110+
});
101111
});

0 commit comments

Comments
 (0)