jingram/cpm-stats-on-ntp-with-scroll #2073
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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)
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.
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
✅ Deploy Preview for content-scope-scripts ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jsoningram
changed the title
[Beta] Generated file diffTime updated: Thu, 04 Dec 2025 21:47:39 GMT Apple
File has changed Integration
File has changed Windows
File has changed |
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
These changes are not needed until we decide if we will keep the blocked counts aligned with native
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp-with-scroll
branch
from
437c876 to
ae0af36
Compare
jsoningram
requested review from
a team,
mgurgel and
shakyShane
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
special-pages/pages/new-tab/app/protections/utils/useAnimatedCount.js
Outdated
Show resolved
Hide resolved
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.
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.
More linter issues
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp-with-scroll
branch
from
e826b16 to
5f50fb7
Compare
shakyShane
approved these changes
Dec 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Asana Task/Github Issue: Allow requesting NTP to scroll widgets into focus
Description
Adds messaging subscription to automatically scroll to the protections report heading when the
protections_scrollmessage is received and minor styling adjustments requested during the ship review. Late addition to this PR is the replacement of theNewBadgeIconwith a component that accepts translations.cpm-scroll.mov
Testing Steps
To simply view the new UI and review the style edits called out in the ship review visit the preview deployment
To test the scrollto functionality…
michal/cpm-on-ntp-4-dialogDuckDuckGo.xcworkspacefile from the Apple Browsers repo (located at the root level)npm installe.g.:npm i github:duckduckgo/content-scope-scripts#57121485acd9b785b00ef89e78063e5c7e173919 && npm installCPM->Show feature awareness dialog for NTP widgetprotections_scrollmessage.Checklist
Please tick all that apply:
Note
Adds a subscription-triggered scroll to the Protections heading, animates counts only when in viewport, replaces the badge SVG with a translatable NewBadge component, and updates styles, strings, types, and tests.
ProtectionsHeading: subscribes toprotections_scrolland scrolls into view; uses refs and integratesNewBadgefor cookie popup stats; minor layout tweaks.useAnimatedCount: adds IntersectionObserver-based viewport awareness; snaps/continues on visibility changes; maintains last seen value.animateCount: minor performance optimizations; ensures exact final value; caps and thresholds retained.NewBadgeIconwith translatableNewBadge(NewBadge.js+NewBadge.module.css).protections_scroll.subscribe.jsonandProtectionsScrollSubscriptiontotypes/new-tab.ts.TickPillspacing/radius; adjustPrivacyStatsinfo icon margin; reduceProtectionsblock top margin.protections_newBadgekey and translations; trims tooltip copy across locales.animateCounttests with improved RAF/time mocks and snapshot validation.Written by Cursor Bugbot for commit 3e6994b. This will update automatically on new commits. Configure here.