Skip to content

Rememes - Media Sizes - Notification fixes#2375

Merged
prxt6529 merged 4 commits into
mainfrom
rememe-media-notification-fixes
May 13, 2026
Merged

Rememes - Media Sizes - Notification fixes#2375
prxt6529 merged 4 commits into
mainfrom
rememe-media-notification-fixes

Conversation

@prxt6529
Copy link
Copy Markdown
Collaborator

@prxt6529 prxt6529 commented May 13, 2026

Summary by CodeRabbit

  • New Features

    • Intrinsic image-height support for improved responsive media rendering
    • Rememe submission flow now posts via a consolidated helper and surfaces normalized error messages (with conditional staging auth)
  • Bug Fixes

    • Notifications no longer default missing vote values
    • Removed fixed-height image wrappers and improved image retry/fallback behavior
  • Tests

    • Expanded coverage for image rendering, notifications, and rememe submission workflows

Review Change Stack

prxt6529 added 2 commits May 13, 2026 09:18
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

This PR adds an intrinsicHeight mode for drop images, makes AllDrops notification vote optional, moves rememe submissions from postData to fetch with staging auth and normalized errors, and refactors Rememes fetching to use abortable, memoized requests with URL sync and pagination reset logic.

Changes

Intrinsic Image Height Feature

Layer / File(s) Summary
Image component implementation and integration
components/drops/view/item/content/media/DropListItemContentMediaImage.tsx, components/drops/view/part/DropPartMarkdownImage.tsx
DropListItemContentMediaImage adds optional intrinsicHeight prop that enables native <img> rendering (currentSrc) with one-shot fallback to unscaled src, retry/backoff handling, and container height switching (tw-min-h-40 vs tw-h-full). DropPartMarkdownImage removes fixed tw-h-64 and passes intrinsicHeight.
Intrinsic height test coverage
__tests__/components/DropListItemContentMediaImage.test.tsx, __tests__/components/drops/view/part/DropPartMarkdownImage.test.tsx
Tests assert wrapper classes, image sizing classes, absence of Next.js fill, removal of legacy fixed-height class, and that intrinsic-mode image errors schedule retry (setTimeout 500ms) instead of immediate fallback.

AllDrops Notification Vote Context

Layer / File(s) Summary
Type and API contract for optional vote
types/feed.types.ts, services/api/notifications-v2-api.ts
INotificationAllDrops.additional_context.vote is now optional; mapNotificationV2 includes vote only when context.vote is a number, omitting it otherwise.
Vote absence and default behavior tests
__tests__/services/api/notifications-v2-api.test.ts, __tests__/components/brain/notifications/all-drops/NotificationAllDrops.test.tsx
Tests verify that an empty additional_context is preserved and UI renders "posted" without showing "reset rating to 0".

Rememe Submission API Migration

Layer / File(s) Summary
RememeAddPage fetch integration and helpers
components/rememes/RememeAddPage.tsx, components/rememes/RememeAddComponent.tsx
RememeAddPage now posts via postRememeSubmission (uses fetchUrl), optionally includes x-6529-auth from getStagingAuth, and normalizes errors via getSubmissionErrorMessage. Submission effect now depends on addRememe. Spinner and Validate button markup were adjusted.
Fetch mocking and submission scenarios
__tests__/components/rememes/RememeAddPage.test.tsx
Test suite replaces postData mocks with global.fetch mocks, adds getStagingAuth mock, updates window.location setup, and adds tests for pending, success, per-NFT errors, backend failure (Insufficient TDH), and add-another flows using fetch assertions.

Rememes Component Fetch/URL/Filter Refactoring

Layer / File(s) Summary
Fetch request management with abort control
components/rememes/Rememes.tsx
Introduces parsedQueryMemeId, an activeFetchRequest AbortController ref, and a memoized fetchResults that aborts prior requests, passes abort signal to fetchUrl, and guards state updates against stale responses.
URL state sync and filter change detection
components/rememes/Rememes.tsx
Adds an effect to keep meme_id search param in sync with selectedMeme via router.replace, consolidates fetch/page-reset logic into a single effect that resets page on filter changes using a previousFilters ref, and cleans up active requests.
Loader styling update & tests
components/rememes/Rememes.module.scss, components/rememes/RememeAddComponent.tsx, __tests__/components/rememes/Rememes.test.tsx
.validateButton and .loaderSlot classes were added; .loader size and alignment adjusted; tests add mockRouterReplace and assert fetchUrl calls include an options object containing an abort signal alongside sort params.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • analyticsflowee

Poem

🐰 I hop where images learn to stand tall,
Votes tidy up when contexts shrink small,
Fetch carries rememes with headers in tow,
Abortors tidy the fetch ebb and flow,
Hooray — a little rabbit clap for the PR!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title broadly covers three main areas of change: rememes, media sizes, and notifications. It accurately reflects the changeset which includes rememe submission flow updates, image sizing logic changes, and notification context handling—though the title is somewhat general and doesn't highlight the specific focus areas.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rememe-media-notification-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
__tests__/components/DropListItemContentMediaImage.test.tsx (1)

91-103: ⚡ Quick win

Add a regression test for intrinsic error handling when primarySrc equals src.

This test suite currently misses the failure mode where fallback swapping is a no-op; adding that case would lock in retry/error behavior for intrinsic mode.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@__tests__/components/DropListItemContentMediaImage.test.tsx` around lines 91
- 103, Add a regression test in
__tests__/components/DropListItemContentMediaImage.test.tsx that simulates an
image load error when primarySrc === src and intrinsicHeight is true;
specifically add an it block referencing the DropListItemContentMediaImage
component with props src and primarySrc set to the same value and
intrinsicHeight, trigger an error event on the rendered img and assert that the
fallback swap is a no-op (no infinite retry) and the component transitions to
the expected error/fallback state (e.g., shows the fallback UI or sets an error
flag) rather than repeatedly attempting to swap sources; this ensures intrinsic
error handling when primarySrc equals src is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@__tests__/services/api/notifications-v2-api.test.ts`:
- Around line 234-237: The assertion using toMatchObject allows extra keys in
additional_context (e.g., vote) to slip through; update the test that inspects
the notification variable so it performs a strict check on
additional_context—either replace the toMatchObject check with a direct equality
assertion on notification?.additional_context toEqual({}) or add an explicit
assertion that notification?.additional_context does not have the "vote"
property (not.toHaveProperty("vote")) to ensure vote isn't being added when not
provided.

In `@components/drops/view/item/content/media/DropListItemContentMediaImage.tsx`:
- Around line 135-143: The intrinsic error handler (handleIntrinsicImageError)
currently swaps to a fallback by toggling usedFallback and setCurrentSrc(src)
even when the primary/fallback URL equals the current src, which prevents
advancing retry/error state; change the logic so that when !usedFallback you
only call setCurrentSrc/setUsedFallback if the fallback URL is different from
the current src (compare primarySrc or fallbackSrc to src), otherwise call
handleError() immediately to advance retry state; update the dependency array to
include any new symbols you reference (e.g., primarySrc or fallbackSrc) and keep
references to setCurrentSrc, setUsedFallback, usedFallback, handleError intact.

---

Nitpick comments:
In `@__tests__/components/DropListItemContentMediaImage.test.tsx`:
- Around line 91-103: Add a regression test in
__tests__/components/DropListItemContentMediaImage.test.tsx that simulates an
image load error when primarySrc === src and intrinsicHeight is true;
specifically add an it block referencing the DropListItemContentMediaImage
component with props src and primarySrc set to the same value and
intrinsicHeight, trigger an error event on the rendered img and assert that the
fallback swap is a no-op (no infinite retry) and the component transitions to
the expected error/fallback state (e.g., shows the fallback UI or sets an error
flag) rather than repeatedly attempting to swap sources; this ensures intrinsic
error handling when primarySrc equals src is covered.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68834a90-6d48-43b6-ab8e-3caab6e11350

📥 Commits

Reviewing files that changed from the base of the PR and between 416b9de and a8ba8a4.

📒 Files selected for processing (14)
  • __tests__/components/DropListItemContentMediaImage.test.tsx
  • __tests__/components/brain/notifications/all-drops/NotificationAllDrops.test.tsx
  • __tests__/components/drops/view/part/DropPartMarkdownImage.test.tsx
  • __tests__/components/rememes/RememeAddPage.test.tsx
  • __tests__/components/rememes/Rememes.test.tsx
  • __tests__/services/api/notifications-v2-api.test.ts
  • components/drops/view/item/content/media/DropListItemContentMediaImage.tsx
  • components/drops/view/part/DropPartMarkdownImage.tsx
  • components/rememes/RememeAddComponent.tsx
  • components/rememes/RememeAddPage.tsx
  • components/rememes/Rememes.module.scss
  • components/rememes/Rememes.tsx
  • services/api/notifications-v2-api.ts
  • types/feed.types.ts

Comment thread __tests__/services/api/notifications-v2-api.test.ts Outdated
Comment thread components/drops/view/item/content/media/DropListItemContentMediaImage.tsx Outdated
prxt6529 added 2 commits May 13, 2026 09:37
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
@sonarqubecloud
Copy link
Copy Markdown

@prxt6529 prxt6529 changed the title Rememe media notification fixes Rememes - Media Sizes - Notification fixes May 13, 2026
@prxt6529 prxt6529 merged commit c1b1acf into main May 13, 2026
8 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 14, 2026
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