Skip to content

fix: keep below-threshold label only on cards#5830

Merged
nimrodkra merged 5 commits intomainfrom
feat/card-only-upvote-label-visibility-clean
Apr 1, 2026
Merged

fix: keep below-threshold label only on cards#5830
nimrodkra merged 5 commits intomainfrom
feat/card-only-upvote-label-visibility-clean

Conversation

@nimrodkra
Copy link
Copy Markdown
Member

@nimrodkra nimrodkra commented Mar 31, 2026

Summary

  • Keep belowThresholdLabel rendering only on card surfaces.
  • Remove below-threshold label fallback rendering from post page, repost modal, and extension companion.
  • Add focused regression tests to ensure non-card surfaces never render the label fallback.

Test plan

  • node ./scripts/typecheck-strict-changed.js
  • NODE_ENV=test pnpm --filter shared exec jest src/components/post/PostUpvotesCommentsCount.spec.tsx src/components/modals/RepostListItem.spec.tsx
  • NODE_ENV=test pnpm --filter extension exec jest src/companion/CompanionEngagements.spec.tsx

Made with Cursor

Preview domain

https://feat-card-only-upvote-label-visi.preview.app.daily.dev

Remove below-threshold label rendering from post page, repost modal, and extension companion while preserving card surfaces and adding focused non-card regression tests.

Made-with: Cursor
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 31, 2026

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

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Apr 1, 2026 8:52am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
storybook Ignored Ignored Apr 1, 2026 8:52am

Request Review

@nimrodkra
Copy link
Copy Markdown
Member Author

@claude please review this pr

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Mar 31, 2026

Claude finished @nimrodkra's task in 1m 43s —— View job


PR Review

  • Gathering context
  • Reading changed files
  • Analyzing changes
  • Posting review

Overall, this is a clean, well-scoped PR. The intent is clear and the implementation is consistent across all three surfaces. A few things worth flagging:


RepostListItem.tsx — Orphan icon when below threshold

packages/shared/src/components/modals/RepostListItem.tsx:143-147

<span className="flex items-center gap-1.5">
  <UpvoteIcon className="size-4" />
  {showUpvotes ? largeNumberFormat(upvotes) : ''}
</span>

When showUpvotes is false, the UpvoteIcon still renders with an empty string beside it. This means a lone upvote icon appears in the repost modal with no count or label — which may look unintentional. If the design intent is to show nothing below threshold, the whole <span> should be conditional:

{showUpvotes && (
  <span className="flex items-center gap-1.5">
    <UpvoteIcon className="size-4" />
    {largeNumberFormat(upvotes)}
  </span>
)}

Fix this →


Minor: belowThresholdLabel still passed to getUpvoteCountDisplay but never used

In all three modified components, upvoteThresholdConfig.belowThresholdLabel is still forwarded as the third argument to getUpvoteCountDisplay, even though the returned belowThresholdLabel is no longer destructured. This is harmless but slightly inconsistent — the parameter is doing nothing at these call sites now. Not blocking, just worth noting.


Tests look good

The three new spec files are tightly scoped regression tests — exactly the right level of coverage for "this surface must never show the below-threshold label." The mock setup is correct and the tests are isolated cleanly. One small note: CompanionEngagements.spec.tsx mocks useQueryClient returning { setQueryData: jest.fn() }, which is correct since the component calls useQueryClient() directly even though useRawBackgroundRequest is also mocked — no issue there.


Summary: The core change is correct and consistent. The main thing to address before merge is the orphan icon in RepostListItem (or confirm it's intentional per design). The belowThresholdLabel passthrough cleanup is optional but worth a follow-up.

Remove the orphan upvote icon in repost modal items by rendering the upvote row only when the numeric count is visible, and extend regression coverage.

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

nimrodkra commented Mar 31, 2026

Addressed, thanks.

  • Wrapped the repost upvote row in showUpvotes so the icon is not rendered when the count is hidden.
  • Updated RepostListItem.spec.tsx to assert both:
    • New is not rendered
    • the upvote row is not rendered (repost-upvotes test id).

Pushed in commit 3a6f799.

Prevent below-threshold upvote labels from appearing in post-page metadata while keeping card metadata behavior unchanged for surfaces that pass explicit upvote counts.

Made-with: Cursor
Apply prettier-consistent indentation in PostMetadata to satisfy shared lint checks.

Made-with: Cursor
@nimrodkra nimrodkra enabled auto-merge (squash) April 1, 2026 08:48
@nimrodkra nimrodkra merged commit 9540608 into main Apr 1, 2026
12 checks passed
@nimrodkra nimrodkra deleted the feat/card-only-upvote-label-visibility-clean branch April 1, 2026 08:52
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