Skip to content

feat: add Happening Now sidebar widget on post pages#5919

Open
nimrodkra wants to merge 3 commits intomainfrom
feat/happening-now-post-widget
Open

feat: add Happening Now sidebar widget on post pages#5919
nimrodkra wants to merge 3 commits intomainfrom
feat/happening-now-post-widget

Conversation

@nimrodkra
Copy link
Copy Markdown
Member

@nimrodkra nimrodkra commented Apr 26, 2026

Summary

Adds a compact "Happening Now" sidebar widget to article, squad and collection post sidebars, covering both standalone post pages and post modals.

  • New HighlightPostSidebarWidget shows a single major headline that rotates every 6s with a fade transition (instant swap when prefers-reduced-motion: reduce); pauses on hover/focus and when the tab is hidden.
  • Headline is clamped to 3 lines with a fixed-height container so layout doesn't shift between short and long headlines.
  • Visual style matches other sidebar widgets (WidgetContainer border, no background, typo-callout title) while keeping the gradient "Happening Now" brand identity.
  • Placed within the discovery cluster (after ShareMobile, before FurtherReading / FeaturedArchives), consistent across all three sidebar files.
  • Gated behind the new post_page_highlights GrowthBook flag, evaluated only for logged-in users via useConditionalFeature.
  • Reuses majorHeadlinesQueryOptions — no backend changes.
  • Logs impression / headline-click / read-all-click via feedHighlightsLogEvent with Origin.ArticlePage.

Test plan

  • Toggle post_page_highlights flag on/off — widget appears/disappears for logged-in users.
  • Article post page and ArticlePostModal both render the widget.
  • Squad post page and SharePostModal both render the widget.
  • Collection post page and CollectionPostModal both render the widget.
  • Headline rotates every ~6s and fades cleanly between transitions.
  • Hovering the widget pauses rotation; mouse-leave resumes it.
  • Long headlines clamp to 3 lines without layout shift.
  • "Read all" link navigates to /highlights?highlight=<firstId>.
  • Reduced-motion OS setting swaps headlines instantly with no fade.
  • Unit tests pass: pnpm --filter @dailydotdev/shared exec jest HighlightPostSidebarWidget.

Made with Cursor

Preview domain

https://feat-happening-now-post-widget.preview.app.daily.dev

Adds a compact, rotating "Happening Now" widget to article, squad and
collection post sidebars (covers post pages and post modals). Gated
behind the new `post_page_highlights` GrowthBook flag.

Made-with: Cursor
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Apr 26, 2026 0:45am
storybook Building Building Preview Apr 26, 2026 0:45am

Request Review

@nimrodkra
Copy link
Copy Markdown
Member Author

@claude please review this pr

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 26, 2026

Claude finished @nimrodkra's task in 2m 23s —— View job


PR Review: feat/happening-now-post-widget

Tasks

  • Gather context and understand request
  • Read changed files
  • Review component implementation
  • Review tests
  • Review feature flag and feed integration
  • Post final review

Summary

Overall this is a clean, well-structured feature. The implementation follows codebase patterns well — proper early returns, useConditionalFeature gating, useLogEventOnce for impressions, and consistent placement across all three post widget files. Accessibility handling (aria-live, reduced-motion) is thoughtful. A few issues worth addressing before merge:


Issues

Medium — Hover-pause test passes for the wrong reason

packages/shared/src/components/cards/highlight/HighlightPostSidebarWidget.spec.tsx:168

// Timer setup happens AFTER hover (when rotation is already paused)
await userEvent.hover(article);   // isPaused = true → interval never created

jest.useFakeTimers();              // No real interval exists at this point
await act(async () => {
  jest.advanceTimersByTime(6000);  // Nothing to advance — this is dead code
  jest.advanceTimersByTime(500);
});

The test passes because isPaused=true prevents the rotation interval from being created at all — not because of any timer manipulation. The jest.useFakeTimers() and advanceTimersByTime calls are misleading dead code. A stronger test would:

  1. Use jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }) from the start (like the rotation test does)
  2. Verify the headline rotates before hover
  3. Hover, advance timers, verify headline does not rotate
  4. Unhover, advance timers, verify rotation resumes

Fix this →


Minor — Redundant highlights.length dependency in rotation effect

packages/shared/src/components/cards/highlight/HighlightPostSidebarWidget.tsx:114

// canRotate is already: shouldRender && highlights.length > 1 && !isPaused
// highlights.length is already baked into canRotate — listing it separately is noise
}, [canRotate, highlights.length]);

canRotate already encapsulates highlights.length > 1, so the explicit highlights.length dep is redundant. The interval callback uses highlights.length for the modulo, but that reads the latest closure value at tick-time regardless of deps. Drop highlights.length from the array.

Fix this →


Minor — No test for impression event firing

The test suite covers rendering, feature-flag gating, rotation, and hover-pause, but never asserts that the useLogEventOnce impression fires when the widget first becomes visible. Given that logEvent is mocked (line 60), a simple assertion would catch regressions:

it('logs an impression when highlights load', async () => {
  mockGqlRequest.mockResolvedValue(buildResponse([buildHighlight('h1', 'Headline')]));
  renderWidget();
  await screen.findByText('Heading Now');
  expect(logEvent).toHaveBeenCalledWith(
    expect.objectContaining({ event_name: 'impression' }),
  );
});

Positives

  • Cleanup is correct: both clearInterval and clearTimeout for the in-flight fade are handled in the effect cleanup (lines 107–113), preventing state updates after unmount.
  • aria-live="polite" on the rotating container is the right ARIA pattern for non-urgent dynamic content.
  • Reduced-motion handled correctly: instant swap, no animation.
  • Tab-hidden guard inside the interval (document.hidden check) is a nice touch.
  • shouldRender early return at line 119 before any JSX, consistent with the codebase's early-return style.
  • origin addition to feedHighlightsLogEvent is backward-compatible (optional param).
  • Placement is consistent across all three widget files and matches the described discovery-cluster position.

- Restructure hover-pause test to fake-time before render so the rotation
  interval is live, then verify rotation actually pauses on hover and
  resumes on unhover (previously the test passed because the interval
  was never created during pause).
- Add a test asserting the impression event fires when highlights load.

Made-with: Cursor
@nimrodkra
Copy link
Copy Markdown
Member Author

Thanks for the review! Addressed two of three findings in 5b0f3b4:

  • Hover-pause test (Medium): Restructured to fake-time before render so the rotation interval is actually live, then verify rotation pauses on hover and resumes on unhover. The previous test was passing for the wrong reason — good catch.
  • Impression test (Minor): Added.

Declined dropping highlights.length from the rotation useEffect deps. The note in the review says the interval callback "reads the latest closure value at tick-time regardless of deps" — that's not quite right. Closures capture variables when the effect body runs, so dropping the dep means the modulo would use a stale highlights.length if the count ever shifts between fetches (unlikely with first: 10, but possible after a stale-while-revalidate update). Keeping the dep is the safer call.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants