Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a marketplace preview subsystem (URL-kind detection, preview state hook, marketplace-specific preview components), NFT-link API and OpenAPI schemas, curation URL validation and UI, drop eligibility logic, WebSocket-driven preview cache sync, many helpers/components, and widespread test updates and prop surface removals (marketplaceImageOnly → nft_links). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MarketplacePreview
participant useMarketplacePreviewState
participant NFT_Link_API
participant LinkPreview_API
participant QueryClient
User->>MarketplacePreview: mount(href)
MarketplacePreview->>useMarketplacePreviewState: requestState(href, mode)
useMarketplacePreviewState->>QueryClient: check cache(key)
alt cached
QueryClient-->>useMarketplacePreviewState: cached data
else not cached
useMarketplacePreviewState->>NFT_Link_API: fetchNftLink(href)
alt nft-link success
NFT_Link_API-->>useMarketplacePreviewState: nft data
useMarketplacePreviewState->>QueryClient: cache(result)
else nft-link missing/error
NFT_Link_API-->>useMarketplacePreviewState: error/null
useMarketplacePreviewState->>LinkPreview_API: fetchLinkPreview(href)
LinkPreview_API-->>useMarketplacePreviewState: og data
useMarketplacePreviewState->>QueryClient: cache(merged)
end
end
useMarketplacePreviewState-->>MarketplacePreview: previewState
alt loading
MarketplacePreview->>User: render placeholder
else error
MarketplacePreview->>User: render unavailable card
else success
MarketplacePreview->>User: render MarketplaceItemPreviewCard
end
sequenceDiagram
participant Viewer
participant WaveLeaderboardEmptyState
participant AuthContext
participant getWaveDropEligibility
participant EmptyStateVariant
Viewer->>WaveLeaderboardEmptyState: render(props)
WaveLeaderboardEmptyState->>AuthContext: get connectedProfile & isLoggedIn
WaveLeaderboardEmptyState->>getWaveDropEligibility: computeEligibility(isLoggedIn,isProxy,isCuration,participation)
getWaveDropEligibility-->>WaveLeaderboardEmptyState: { canCreateDrop, message, link }
alt isCurationWave
WaveLeaderboardEmptyState->>EmptyStateVariant: render WaveLeaderboardCurationEmptyState(canCreateDrop,message,link)
else isMemesWave
WaveLeaderboardEmptyState->>EmptyStateVariant: render WaveLeaderboardMemesEmptyState()
else default
WaveLeaderboardEmptyState->>EmptyStateVariant: render WaveLeaderboardDefaultEmptyState(canCreateDrop,message)
end
Viewer->>EmptyStateVariant: click create
EmptyStateVariant->>WaveLeaderboardEmptyState: call onCreateDrop (if allowed)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In
`@__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx`:
- Around line 90-138: Add a test that explicitly covers URLs with query
parameters and fragments so the contract is explicit: in the
nftMarketplaces.test.tsx use createNftMarketplacesHandler() and call
handler.match(...) with a URL that includes a query string and/or fragment
(e.g., .../1?ref=abc and .../1#section) and assert the expected boolean (true if
you want to allow matching with queries/fragments, or false if you want to
reject them); place it alongside the existing negative cases to document
intended behavior.
In
`@__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx`:
- Around line 99-104: The participation mock in the test using
useWave.mockReturnValue is incomplete and may cause undefined behavior when
getWaveDropEligibility or other logic reads fields; update the mock returned by
useWave in WaveleaderboardHeader.test.tsx (the test that sets isMemesWave:
false, isCurationWave: false) to include the full participation shape used
elsewhere: canSubmitNow, hasReachedLimit, status, and isEligible (matching the
beforeEach and other tests) so the component sees the same fields as other tests
and avoids flaky failures.
In `@__tests__/components/waves/leaderboard/WaveLeaderboardEmptyState.test.tsx`:
- Around line 4-16: The test mock fully replaces the module so SubmissionStatus
becomes undefined; update the jest.mock factory that currently returns only
useWave to preserve real exports and override only useWave by calling
jest.requireActual for the module and merging its exports with a mocked useWave
(i.e., keep SubmissionStatus intact and replace useWave with jest.fn()), then
run the tests to ensure SubmissionStatus.ACTIVE etc. are available to the
WaveLeaderboardEmptyState tests.
In `@components/waves/CreateDropContent.tsx`:
- Around line 1018-1028: The post-upload curation guard (the if block
referencing isCurationDropMode, parts, and setToast/setSubmitting) is redundant
because generateParts already normalizes to a single part with media: [], so add
a brief inline comment above this guard explaining it's intentional defensive
check (defense-in-depth) to catch any unexpected upstream changes and should
remain even though generateParts should make it a no-op; keep the logic as-is
but document its purpose referencing generateParts and isCurationDropMode for
future readers.
- Around line 959-994: The current ternary in prepareAndSubmitDrop sets
curationValidation via `normalizedCurationText ? null :
validateCurationDropInput(curationText)`, which is correct but terse; make it
clearer by first computing the validation result with
`validateCurationDropInput(curationText)` into a variable (e.g., tempValidation)
and then set `curationValidation = normalizedCurationText ? null :
tempValidation` (or explicitly `if (normalizedCurationText) curationValidation =
null else curationValidation = tempValidation`), referencing the existing
symbols `normalizedCurationText`, `validateCurationDropInput`, and
`curationValidation` so the intent is explicit and easier to read.
- Line 501: The hook useWave(wave) is being called in both CreateDrop (parent)
and CreateDropContent (child); hoist the hook to the parent so it is invoked
only once, compute isCurationWave and isMemesWave there, derive
isCurationDropMode in the parent if needed, and pass isCurationWave and
isMemesWave (and isCurationDropMode if used) as props into CreateDropContent;
update CreateDropContent to remove the useWave call and read those values from
props (identify useWave, isCurationWave, isMemesWave, isCurationDropMode,
CreateDrop, and CreateDropContent in the diff to locate the changes).
In `@components/waves/CreateDropInput.tsx`:
- Around line 333-341: The validation message is absolutely positioned (uses
tw-absolute tw-top-full) and can overlap following content like
CreateDropContentRequirements; instead, make the message participate in document
flow by removing the absolute classes and/or ensure its container (the element
rendered by CreateDropInput) is tw-relative and has reserved space (e.g., add
bottom padding or a min-height) so validationHelperText renders below the editor
without overlapping; update the JSX around validationHelperText to drop
tw-absolute tw-top-full and use tw-block tw-mt-1 (or add a wrapper with
tw-relative plus tw-pb-2 / tw-min-h-[Xpx]) and adjust styles where
validationHelperText is referenced to preserve spacing.
In `@components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx`:
- Line 24: The component WaveLeaderboardCurationEmptyState currently hardcodes
levelPhrase = "Level 10"; change this to accept an optional prop (e.g.,
levelPhrase?: string) and use that when provided, otherwise attempt to derive
the phrase from the existing restrictionMessage (e.g., extract the first match
of a simple regex like /Level\s*\d+/) and fall back to rendering the full
restrictionMessage as plain text if no match is found; update the component
signature and any callers to pass the real level when available and replace the
hardcoded levelPhrase variable with the new prop/derived value.
In `@components/waves/leaderboard/header/WaveleaderboardHeader.tsx`:
- Around line 197-213: The outer wrapper div (rendered when isLoggedIn is true)
can be empty if canCreateDrop is false, causing unwanted spacing; update the
conditional so the div only renders when both isLoggedIn and canCreateDrop are
true (e.g., change the conditional that currently uses isLoggedIn to require
isLoggedIn && canCreateDrop), keeping the existing className logic (including
isMemesWave) and children (PrimaryButton, onCreateDrop, PlusIcon) intact so no
empty element is produced.
In `@components/waves/utils/createDropContentSubmission.ts`:
- Around line 1-9: The two helpers disagree on whitespace handling:
shouldUseInitialDropConfig uses !markdown?.length while
hasCurrentDropPartContent uses markdown?.trim().length; update
shouldUseInitialDropConfig to use trimmed length (mirror
hasCurrentDropPartContent) so both treat whitespace-only strings the same (apply
.trim() to markdown in shouldUseInitialDropConfig), ensuring consistent behavior
between shouldUseInitialDropConfig and hasCurrentDropPartContent.
In `@components/waves/utils/validateCurationDropUrl.ts`:
- Around line 113-115: toCanonicalHttpsUrl currently preserves a leading "www."
because normalizeHostname only lowercases/trims; update the canonicalization so
hostnames have the "www." prefix removed before building the URL. Locate
toCanonicalHttpsUrl (and/or normalizeHostname) and strip a leading "www." from
url.hostname (after lowercasing) so that https://opensea.io/... and
https://www.opensea.io/... normalize to the same canonical form; keep other
normalization (lowercasing/trim) intact and add a short comment explaining the
intentional stripping of "www.".
- Around line 1-4: The type CurationUrlValidationResult currently encodes only
the error case (null == success) which is misleading; either rename the type to
CurationUrlValidationError (or CurationUrlValidationErrorResult) to signal it's
error-only, or refactor it into an explicit discriminated union like a success
branch and an error branch so it can be extended later; update references to
CurationUrlValidationResult and any usages in validateCurationDropUrl to match
the new name/shape.
🧹 Nitpick comments (10)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx`: - Around line 90-138: Add a test that explicitly covers URLs with query parameters and fragments so the contract is explicit: in the nftMarketplaces.test.tsx use createNftMarketplacesHandler() and call handler.match(...) with a URL that includes a query string and/or fragment (e.g., .../1?ref=abc and .../1#section) and assert the expected boolean (true if you want to allow matching with queries/fragments, or false if you want to reject them); place it alongside the existing negative cases to document intended behavior. In `@__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx`: - Around line 99-104: The participation mock in the test using useWave.mockReturnValue is incomplete and may cause undefined behavior when getWaveDropEligibility or other logic reads fields; update the mock returned by useWave in WaveleaderboardHeader.test.tsx (the test that sets isMemesWave: false, isCurationWave: false) to include the full participation shape used elsewhere: canSubmitNow, hasReachedLimit, status, and isEligible (matching the beforeEach and other tests) so the component sees the same fields as other tests and avoids flaky failures. In `@components/waves/CreateDropContent.tsx`: - Around line 1018-1028: The post-upload curation guard (the if block referencing isCurationDropMode, parts, and setToast/setSubmitting) is redundant because generateParts already normalizes to a single part with media: [], so add a brief inline comment above this guard explaining it's intentional defensive check (defense-in-depth) to catch any unexpected upstream changes and should remain even though generateParts should make it a no-op; keep the logic as-is but document its purpose referencing generateParts and isCurationDropMode for future readers. - Around line 959-994: The current ternary in prepareAndSubmitDrop sets curationValidation via `normalizedCurationText ? null : validateCurationDropInput(curationText)`, which is correct but terse; make it clearer by first computing the validation result with `validateCurationDropInput(curationText)` into a variable (e.g., tempValidation) and then set `curationValidation = normalizedCurationText ? null : tempValidation` (or explicitly `if (normalizedCurationText) curationValidation = null else curationValidation = tempValidation`), referencing the existing symbols `normalizedCurationText`, `validateCurationDropInput`, and `curationValidation` so the intent is explicit and easier to read. - Line 501: The hook useWave(wave) is being called in both CreateDrop (parent) and CreateDropContent (child); hoist the hook to the parent so it is invoked only once, compute isCurationWave and isMemesWave there, derive isCurationDropMode in the parent if needed, and pass isCurationWave and isMemesWave (and isCurationDropMode if used) as props into CreateDropContent; update CreateDropContent to remove the useWave call and read those values from props (identify useWave, isCurationWave, isMemesWave, isCurationDropMode, CreateDrop, and CreateDropContent in the diff to locate the changes). In `@components/waves/CreateDropInput.tsx`: - Around line 333-341: The validation message is absolutely positioned (uses tw-absolute tw-top-full) and can overlap following content like CreateDropContentRequirements; instead, make the message participate in document flow by removing the absolute classes and/or ensure its container (the element rendered by CreateDropInput) is tw-relative and has reserved space (e.g., add bottom padding or a min-height) so validationHelperText renders below the editor without overlapping; update the JSX around validationHelperText to drop tw-absolute tw-top-full and use tw-block tw-mt-1 (or add a wrapper with tw-relative plus tw-pb-2 / tw-min-h-[Xpx]) and adjust styles where validationHelperText is referenced to preserve spacing. In `@components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx`: - Line 24: The component WaveLeaderboardCurationEmptyState currently hardcodes levelPhrase = "Level 10"; change this to accept an optional prop (e.g., levelPhrase?: string) and use that when provided, otherwise attempt to derive the phrase from the existing restrictionMessage (e.g., extract the first match of a simple regex like /Level\s*\d+/) and fall back to rendering the full restrictionMessage as plain text if no match is found; update the component signature and any callers to pass the real level when available and replace the hardcoded levelPhrase variable with the new prop/derived value. In `@components/waves/leaderboard/header/WaveleaderboardHeader.tsx`: - Around line 197-213: The outer wrapper div (rendered when isLoggedIn is true) can be empty if canCreateDrop is false, causing unwanted spacing; update the conditional so the div only renders when both isLoggedIn and canCreateDrop are true (e.g., change the conditional that currently uses isLoggedIn to require isLoggedIn && canCreateDrop), keeping the existing className logic (including isMemesWave) and children (PrimaryButton, onCreateDrop, PlusIcon) intact so no empty element is produced. In `@components/waves/utils/validateCurationDropUrl.ts`: - Around line 113-115: toCanonicalHttpsUrl currently preserves a leading "www." because normalizeHostname only lowercases/trims; update the canonicalization so hostnames have the "www." prefix removed before building the URL. Locate toCanonicalHttpsUrl (and/or normalizeHostname) and strip a leading "www." from url.hostname (after lowercasing) so that https://opensea.io/... and https://www.opensea.io/... normalize to the same canonical form; keep other normalization (lowercasing/trim) intact and add a short comment explaining the intentional stripping of "www.". - Around line 1-4: The type CurationUrlValidationResult currently encodes only the error case (null == success) which is misleading; either rename the type to CurationUrlValidationError (or CurationUrlValidationErrorResult) to signal it's error-only, or refactor it into an explicit discriminated union like a success branch and an error branch so it can be extended later; update references to CurationUrlValidationResult and any usages in validateCurationDropUrl to match the new name/shape.__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx (1)
90-138: Thorough negative cases — consider adding a query-parameter edge case.The negative tests cover protocol, chain, path structure, subdomain spoofing, and lookalike domains — excellent coverage. One optional gap: URLs with query parameters or fragments (e.g.,
https://opensea.io/item/ethereum/0x.../1?ref=abc) will still match becausenew URL(...).pathnamestrips?/#. If that's intentional, a single positive test documenting it would make the contract explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx` around lines 90 - 138, Add a test that explicitly covers URLs with query parameters and fragments so the contract is explicit: in the nftMarketplaces.test.tsx use createNftMarketplacesHandler() and call handler.match(...) with a URL that includes a query string and/or fragment (e.g., .../1?ref=abc and .../1#section) and assert the expected boolean (true if you want to allow matching with queries/fragments, or false if you want to reject them); place it alongside the existing negative cases to document intended behavior.components/waves/utils/validateCurationDropUrl.ts (2)
113-115:www.prefix is preserved in the canonical URL — potential deduplication concern.
toCanonicalHttpsUrlusesnormalizeHostname, which lowercases and trims but does not strip thewww.prefix. This meanshttps://opensea.io/item/...andhttps://www.opensea.io/item/...produce different canonical forms for the same resource. If these URLs are later compared or deduplicated (e.g., preventing duplicate curation drops), they won't match.If this is intentional, consider adding a brief comment. Otherwise:
Suggested fix to strip www.
const toCanonicalHttpsUrl = (url: URL): string => { - return `https://${normalizeHostname(url.hostname)}${url.pathname}`; + const hostname = normalizeHostname(url.hostname).replace(/^www\./, ""); + return `https://${hostname}${url.pathname}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/validateCurationDropUrl.ts` around lines 113 - 115, toCanonicalHttpsUrl currently preserves a leading "www." because normalizeHostname only lowercases/trims; update the canonicalization so hostnames have the "www." prefix removed before building the URL. Locate toCanonicalHttpsUrl (and/or normalizeHostname) and strip a leading "www." from url.hostname (after lowercasing) so that https://opensea.io/... and https://www.opensea.io/... normalize to the same canonical form; keep other normalization (lowercasing/trim) intact and add a short comment explaining the intentional stripping of "www.".
1-4: The result type encodes only the error case — consider naming it to reflect that.
CurationUrlValidationResultis{ error: true; helperText: string } | null, wherenullmeans success. Theerror: trueliteral is redundant since the non-null branch is always an error. This is fine as-is, but if the type ever needs a success payload, it'll need restructuring. No action needed now — just a note for future extensibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/validateCurationDropUrl.ts` around lines 1 - 4, The type CurationUrlValidationResult currently encodes only the error case (null == success) which is misleading; either rename the type to CurationUrlValidationError (or CurationUrlValidationErrorResult) to signal it's error-only, or refactor it into an explicit discriminated union like a success branch and an error branch so it can be extended later; update references to CurationUrlValidationResult and any usages in validateCurationDropUrl to match the new name/shape.components/waves/leaderboard/header/WaveleaderboardHeader.tsx (1)
197-213: Empty wrapperdivrenders when logged in butcanCreateDropis false.When
isLoggedInis true butcanCreateDropis false, the outer<div>still renders (empty). With the flex gap on the parent (tw-gap-2), this can introduce unwanted spacing.Suggested fix
- {isLoggedIn && ( - <div - className={`tw-flex tw-flex-col tw-items-end ${isMemesWave ? "lg:tw-hidden" : ""}`} - > - {canCreateDrop && ( - <PrimaryButton - loading={false} - disabled={false} - onClicked={onCreateDrop} - padding="tw-px-3 tw-py-2" - > - <PlusIcon className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" /> - <span>Drop</span> - </PrimaryButton> - )} + {isLoggedIn && canCreateDrop && ( + <div + className={`tw-flex tw-flex-col tw-items-end ${isMemesWave ? "lg:tw-hidden" : ""}`} + > + <PrimaryButton + loading={false} + disabled={false} + onClicked={onCreateDrop} + padding="tw-px-3 tw-py-2" + > + <PlusIcon className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" /> + <span>Drop</span> + </PrimaryButton> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/header/WaveleaderboardHeader.tsx` around lines 197 - 213, The outer wrapper div (rendered when isLoggedIn is true) can be empty if canCreateDrop is false, causing unwanted spacing; update the conditional so the div only renders when both isLoggedIn and canCreateDrop are true (e.g., change the conditional that currently uses isLoggedIn to require isLoggedIn && canCreateDrop), keeping the existing className logic (including isMemesWave) and children (PrimaryButton, onCreateDrop, PlusIcon) intact so no empty element is produced.components/waves/CreateDropContent.tsx (3)
1018-1028: Post-upload curation guard is redundant but acceptable as defense-in-depth.Lines 1018-1028 re-check that curation drops have exactly one part with no media after
generateParts. Since the normalization at Lines 984-993 already ensuresmedia: []and a single part,generatePartsshould be a no-op for media. This guard can never fire under normal flow. Fine to keep for safety, but worth a brief inline comment explaining it's a defensive check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropContent.tsx` around lines 1018 - 1028, The post-upload curation guard (the if block referencing isCurationDropMode, parts, and setToast/setSubmitting) is redundant because generateParts already normalizes to a single part with media: [], so add a brief inline comment above this guard explaining it's intentional defensive check (defense-in-depth) to catch any unexpected upstream changes and should remain even though generateParts should make it a no-op; keep the logic as-is but document its purpose referencing generateParts and isCurationDropMode for future readers.
959-994: Curation normalization logic inprepareAndSubmitDropis sound but the validation inversion is subtle.Line 965-967: the ternary
normalizedCurationText ? null : validateCurationDropInput(curationText)determines the validation error only when normalization fails. This is logically correct (normalization success implies validity), but the inverted conditional makes it easy to misread.A minor clarity improvement:
Optional: make intent clearer
- const curationValidation = normalizedCurationText - ? null - : validateCurationDropInput(curationText); + const curationValidation = !normalizedCurationText + ? validateCurationDropInput(curationText) + : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropContent.tsx` around lines 959 - 994, The current ternary in prepareAndSubmitDrop sets curationValidation via `normalizedCurationText ? null : validateCurationDropInput(curationText)`, which is correct but terse; make it clearer by first computing the validation result with `validateCurationDropInput(curationText)` into a variable (e.g., tempValidation) and then set `curationValidation = normalizedCurationText ? null : tempValidation` (or explicitly `if (normalizedCurationText) curationValidation = null else curationValidation = tempValidation`), referencing the existing symbols `normalizedCurationText`, `validateCurationDropInput`, and `curationValidation` so the intent is explicit and easier to read.
501-501: Avoid callinguseWave(wave)in both parent and child components; hoist the hook call to the parent.
useWave(wave)is called in bothCreateDrop.tsx(line 67) and its childCreateDropContent.tsx(line 501). Both extractisCurationWaveand separately computeisCurationDropModeusing the same derived values. While the hook internally memoizes expensive calculations (pause filtering, submission status, eligibility checks), the redundant hook invocation across the component hierarchy is unnecessary. MoveuseWave(wave)to the parent and passisCurationWaveandisMemesWavedown as props to eliminate the duplicate call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropContent.tsx` at line 501, The hook useWave(wave) is being called in both CreateDrop (parent) and CreateDropContent (child); hoist the hook to the parent so it is invoked only once, compute isCurationWave and isMemesWave there, derive isCurationDropMode in the parent if needed, and pass isCurationWave and isMemesWave (and isCurationDropMode if used) as props into CreateDropContent; update CreateDropContent to remove the useWave call and read those values from props (identify useWave, isCurationWave, isMemesWave, isCurationDropMode, CreateDrop, and CreateDropContent in the diff to locate the changes).__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx (1)
99-104: Participation mock is missing fields compared to other test cases.At Line 103, the participation mock only provides
{ isEligible: true }while thebeforeEach(Line 56-61) and the new tests (Lines 216-221, 254-259) provide a fuller shape (canSubmitNow,hasReachedLimit,status). IfgetWaveDropEligibilityaccesses those fields, this could lead toundefinedvalues and potentially flaky behavior.Suggested fix
useWave.mockReturnValue({ isMemesWave: false, isCurationWave: false, - participation: { isEligible: true }, + participation: { + isEligible: true, + canSubmitNow: true, + hasReachedLimit: false, + status: "ACTIVE", + }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx` around lines 99 - 104, The participation mock in the test using useWave.mockReturnValue is incomplete and may cause undefined behavior when getWaveDropEligibility or other logic reads fields; update the mock returned by useWave in WaveleaderboardHeader.test.tsx (the test that sets isMemesWave: false, isCurationWave: false) to include the full participation shape used elsewhere: canSubmitNow, hasReachedLimit, status, and isEligible (matching the beforeEach and other tests) so the component sees the same fields as other tests and avoids flaky failures.components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx (1)
24-24: Hardcoded"Level 10"for message splitting.The string
"Level 10"is hardcoded for the purpose of bolding it within the restriction message. If the level requirement changes, this presentation logic would silently stop working (graceful fallback to plain text, but the visual emphasis is lost). Consider extracting this as a prop or deriving it from the restriction message if feasible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx` at line 24, The component WaveLeaderboardCurationEmptyState currently hardcodes levelPhrase = "Level 10"; change this to accept an optional prop (e.g., levelPhrase?: string) and use that when provided, otherwise attempt to derive the phrase from the existing restrictionMessage (e.g., extract the first match of a simple regex like /Level\s*\d+/) and fall back to rendering the full restrictionMessage as plain text if no match is found; update the component signature and any callers to pass the real level when available and replace the hardcoded levelPhrase variable with the new prop/derived value.components/waves/CreateDropInput.tsx (1)
333-341: Absolute positioning of validation message may overlap subsequent content.The validation helper text uses
tw-absolute tw-top-fullwhich positions it outside the normal document flow, below the editor. If there's content immediately below the input (e.g.,CreateDropContentRequirements), the message could visually overlap it. Ensure the parent container has sufficient bottom margin/padding, or consider using relative positioning with a min-height reserve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropInput.tsx` around lines 333 - 341, The validation message is absolutely positioned (uses tw-absolute tw-top-full) and can overlap following content like CreateDropContentRequirements; instead, make the message participate in document flow by removing the absolute classes and/or ensure its container (the element rendered by CreateDropInput) is tw-relative and has reserved space (e.g., add bottom padding or a min-height) so validationHelperText renders below the editor without overlapping; update the JSX around validationHelperText to drop tw-absolute tw-top-full and use tw-block tw-mt-1 (or add a wrapper with tw-relative plus tw-pb-2 / tw-min-h-[Xpx]) and adjust styles where validationHelperText is referenced to preserve spacing.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/components/waves/leaderboard/grid/WaveLeaderboardGridItem.test.tsx (1)
110-127:⚠️ Potential issue | 🟠 Major
baseDropis missing anauthorfield — all tests will throw after the?.removal in the production component.
WaveLeaderboardGridItem.tsx(line 112) was changed fromauthor?.handle ?? nulltoauthor.handle ?? null. BecausebaseDrophere has noauthorproperty,drop.authorresolves toundefined, and every render will throw:TypeError: Cannot read properties of undefined (reading 'handle')Fix
baseDropto include a minimal author stub (or see the fix suggestion on the production file):🛠 Proposed fix – add `author` to `baseDrop`
const baseDrop: any = { id: "d1", rank: 1, drop_type: "PARTICIPATORY", metadata: [], + author: { handle: "testuser" }, parts: [ { media: [{ url: "media", mime_type: "image/jpeg" }], content: "hello", }, ], wave: { id: "w1" }, context_profile_context: { curatable: true, curated: false }, mentioned_users: [], mentioned_waves: [], referenced_nfts: [], winning_context: { decision_time: null }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/grid/WaveLeaderboardGridItem.test.tsx` around lines 110 - 127, The test fixture baseDrop in WaveLeaderboardGridItem.test.tsx is missing an author object which will cause renders to throw after the production change from author?.handle to author.handle; update the baseDrop object to include a minimal author stub (e.g., at least an author.handle string, and optionally author.id) so drop.author is defined in tests and WaveLeaderboardGridItem can access author.handle without error.
🧹 Nitpick comments (37)
__tests__/components/brain/my-stream/MyStreamWaveLeaderboard.test.tsx (2)
57-67:data-curation-leaderboardattribute is set but never asserted in tests.The mock renders
data-curation-leaderboard={String(Boolean(props.isCurationLeaderboard))}on line 63, but all assertions use thecreateDropPropsarray instead. The attribute is redundant—either add an assertion usinggetAttribute('data-curation-leaderboard')or remove the attribute to keep the mock lean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/my-stream/MyStreamWaveLeaderboard.test.tsx` around lines 57 - 67, The mock for WaveDropCreate currently renders a data-curation-leaderboard attribute but no tests assert it; either remove that attribute from the WaveDropCreate mock to keep the mock minimal, or add an assertion in the test that mounts the mock which reads the rendered node's getAttribute('data-curation-leaderboard') and compares it to String(Boolean(createDropProps[i].isCurationLeaderboard)); update the test code that uses the createDropProps array to also query the DOM for data-testid="create-drop" and assert the attribute when choosing the assertion route.
108-124: Consider adding a test for ineligible participation state.All test cases mock
participationas fully eligible (isEligible: true, canSubmitNow: true, hasReachedLimit: false). A test verifying thatonCreateDrop/ the create-drop UI is hidden or disabled when the user is ineligible (e.g.,canSubmitNow: false) would strengthen confidence in the eligibility gating logic within this component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/my-stream/MyStreamWaveLeaderboard.test.tsx` around lines 108 - 124, Add a test that mocks the component's participation state to an ineligible case (e.g., set canSubmitNow: false or isEligible: false on the participation mock returned to the component) and assert that the create-drop UI is hidden/disabled and that the onCreateDrop handler (or create button click) is not invoked; update the test suite in MyStreamWaveLeaderboard.test.tsx to use the same render helpers and mocks you already use (mocking the participation hook or prop used by MyStreamWaveLeaderboard) and verify absence/disabled state of the create control and that onCreateDrop is not called.__tests__/services/api/nft-link-api.test.ts (1)
6-9:jest.clearAllMocks()is redundant beforefetchMock.mockReset().
mockReset()is a strict superset ofclearAllMocks()— it clears call history and resets implementation/return values. The precedingjest.clearAllMocks()call adds no effect.♻️ Proposed simplification
beforeEach(() => { - jest.clearAllMocks(); fetchMock.mockReset(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/services/api/nft-link-api.test.ts` around lines 6 - 9, Remove the redundant jest.clearAllMocks() call in the test setup: keep fetchMock.mockReset() in the beforeEach block since fetchMock.mockReset() already clears call history and resets implementations, making jest.clearAllMocks() unnecessary; update the beforeEach that currently references jest.clearAllMocks and fetchMock.mockReset to only call fetchMock.mockReset() (locate the beforeEach in __tests__/services/api/nft-link-api.test.ts).components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx (1)
117-117: Internal variableisMarketplaceImageOnlyCardstill uses the oldImageOnlynaming.The prop has been renamed to
marketplaceCompact, but the internal memoized boolean keeps the old name. This is a minor naming inconsistency within the file.♻️ Rename suggestion
- const isMarketplaceImageOnlyCard = useMemo(() => { + const isMarketplaceCompactCard = useMemo(() => { ... - }, [activePart?.content, mode, primaryMedia]); + }, [activePart?.content, mode, primaryMedia]); - if (isMarketplaceImageOnlyCard) { + if (isMarketplaceCompactCard) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx` at line 117, The internal memoized boolean is still named isMarketplaceImageOnlyCard but the prop was renamed to marketplaceCompact; update the useMemo declaration and every usage inside the WaveLeaderboardGridItem component to isMarketplaceCompact (or a similar name matching marketplaceCompact) so the variable name reflects the prop, e.g., change const isMarketplaceImageOnlyCard = useMemo(...) to const isMarketplaceCompact = useMemo(...) and replace all references to isMarketplaceImageOnlyCard accordingly while keeping the existing logic unchanged.__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx (1)
140-158: Consider adding a default (no-options) render assertion.The rendering test only covers
marketplaceCompact: true. A complementary assertion for the no-options call (wherecompactisundefined) would protect theoptions?.marketplaceCompactoptional-chain path increateNftMarketplacesHandler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx` around lines 140 - 158, Add a complementary test that calls createNftMarketplacesHandler() with no options to exercise the options?.marketplaceCompact path: create a handler via createNftMarketplacesHandler(), call handler.render(href) with the same sample href, render the returned element, and assert mockMarketplacePreview was called with { href, compact: undefined } (or without the compact prop) and that the rendered node with test id "marketplace-preview" still has data-href equal to href; this ensures the default/no-options behavior is covered alongside the existing marketplaceCompact: true case.components/waves/marketplace/MarketplaceFoundationMintPreview.tsx (1)
1-40: Extract shared rendering logic —MarketplaceFoundationMintPreview,MarketplaceManifoldListingPreview, andMarketplaceOpenseaAssetPrevieware near-identical.All three components share the same state-check + render-path pattern.
MarketplaceOpenseaAssetPreviewdiffers only by passingmode: "opensea-sanitized"touseMarketplacePreviewState. This is a DRY violation across three files.A single generic component could eliminate the duplication:
♻️ Proposed shared base component
// components/waves/marketplace/MarketplaceBasePreview.tsx +"use client"; + +import MarketplaceItemPreviewCard from "../MarketplaceItemPreviewCard"; +import MarketplacePreviewPlaceholder from "./MarketplacePreviewPlaceholder"; +import MarketplaceUnavailableCard from "./MarketplaceUnavailableCard"; +import type { MarketplaceTypePreviewProps } from "./common"; +import { useMarketplacePreviewState } from "./useMarketplacePreviewState"; + +interface MarketplaceBasePreviewProps extends MarketplaceTypePreviewProps { + readonly mode?: string; +} + +export default function MarketplaceBasePreview({ + href, + compact = false, + mode, +}: MarketplaceBasePreviewProps) { + const state = useMarketplacePreviewState({ href, ...(mode ? { mode } : {}) }); + + if (state.href !== href || state.type === "loading") { + return <MarketplacePreviewPlaceholder href={href} compact={compact} />; + } + if (state.type === "error") { + return <MarketplaceUnavailableCard href={href} compact={compact} />; + } + const media = state.resolvedMedia; + if (media) { + return ( + <MarketplaceItemPreviewCard + href={href} + mediaUrl={media.url} + mediaMimeType={media.mimeType} + price={state.resolvedPrice} + title={state.resolvedTitle} + compact={compact} + hideActions={compact} + /> + ); + } + return <MarketplaceUnavailableCard href={href} compact={compact} />; +}Then each specific preview becomes a thin wrapper (or the router
MarketplacePreviewdispatches directly with the rightmode):// MarketplaceFoundationMintPreview.tsx -export default function MarketplaceFoundationMintPreview({ href, compact = false }: MarketplaceTypePreviewProps) { - const state = useMarketplacePreviewState({ href }); - // ... 30 lines of shared logic -} +export { default } from "./MarketplaceBasePreview"; // MarketplaceOpenseaAssetPreview.tsx -export default function MarketplaceOpenseaAssetPreview({ href, compact = false }: MarketplaceTypePreviewProps) { - const state = useMarketplacePreviewState({ href, mode: "opensea-sanitized" }); - // ... 30 lines of shared logic -} +import MarketplaceBasePreview from "./MarketplaceBasePreview"; +export default function MarketplaceOpenseaAssetPreview(props: MarketplaceTypePreviewProps) { + return <MarketplaceBasePreview {...props} mode="opensea-sanitized" />; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceFoundationMintPreview.tsx` around lines 1 - 40, Multiple near-identical components (MarketplaceFoundationMintPreview, MarketplaceManifoldListingPreview, MarketplaceOpenseaAssetPreview) duplicate the same state-check and render logic; refactor by extracting a single generic Preview component that accepts props like href, compact, and an optional mode (passed into useMarketplacePreviewState) and encapsulates the shared flow (calling useMarketplacePreviewState, returning MarketplacePreviewPlaceholder when loading/mismatched href, MarketplaceUnavailableCard on error or missing media, and MarketplaceItemPreviewCard when media exists). Replace the three specific components with thin wrappers that call the generic Preview with the appropriate mode (e.g., mode: "opensea-sanitized") or export the generic from the router so callers pass mode, keeping existing components MarketplacePreviewPlaceholder, MarketplaceUnavailableCard, and MarketplaceItemPreviewCard usage unchanged.services/api/nft-link-api.ts (1)
26-34: Type guard validates only property existence, not field types.
isApiNftLinkResponsereturnstruefor any object with the three property keys, regardless of value types — e.g.,{ is_enrichable: "yes", validation_error: 42, data: [] }passes. For a trusted internal API this is usually acceptable, butis_enrichablebeingbooleanis load-bearing for downstream rendering decisions.♻️ Stricter guard
const isApiNftLinkResponse = (value: unknown): value is ApiNftLinkResponse => { if (value === null || typeof value !== "object" || Array.isArray(value)) { return false; } - - return ( - "is_enrichable" in value && "validation_error" in value && "data" in value - ); + const v = value as Record<string, unknown>; + return ( + typeof v["is_enrichable"] === "boolean" && + ("validation_error" in v) && + ("data" in v) + ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/api/nft-link-api.ts` around lines 26 - 34, The current type guard isApiNftLinkResponse only checks keys; update it to validate field types as well: ensure is_enrichable is a boolean, validation_error is either null or a string (or the expected type your API uses), and data is present and of the expected shape (at minimum not undefined—refine further if you know it should be an object/array). Adjust the return expression in isApiNftLinkResponse to perform these typeof/null/Array.isArray checks so the guard only returns true for correctly typed responses.services/websocket/WebSocketProvider.tsx (1)
32-39: DuplicateasNonEmptyStringutility.An identical
asNonEmptyStringfunction exists inapp/api/open-graph/opensea/shared.ts(lines 139-146). Consider extracting to a shared utility module to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/WebSocketProvider.tsx` around lines 32 - 39, Duplicate asNonEmptyString utility found in WebSocketProvider.tsx; extract it into a shared utility module and import it from both locations. Create a new helper (e.g., asNonEmptyString) in a common utils file (e.g., services/utils/string.ts or app/shared/string.ts), move the implementation there, update WebSocketProvider.tsx to import and use that exported function instead of the local definition, and update app/api/open-graph/opensea/shared.ts to import the same exported function so both files reuse the single implementation.components/waves/utils/getOptimisticDrop.ts (1)
1-1:"use client"directive is unnecessary for a pure utility function.This file exports a plain function with no React hooks or components. The directive has no effect here and could be removed for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/getOptimisticDrop.ts` at line 1, Remove the top-level "use client" directive from this utility module since it only exports the plain function getOptimisticDrop (and contains no React components or hooks); locate the `getOptimisticDrop` export in components/waves/utils/getOptimisticDrop.ts and delete the `"use client"` line, verifying no other client-only code remains.__tests__/services/websocket/WebSocketProvider.test.tsx (1)
81-86: Mock setup order:mockReturnValuebeforeclearAllMocksis fine but potentially confusing.
jest.clearAllMocks()on line 86 clears call counts and results but does not reset mock implementations/return values, so themockReturnValue("fresh-token")from line 84 remains in effect. Tests that need a different return value override it explicitly, which is correct. However, the ordering reads as if the setup might be wiped — consider movingjest.clearAllMocks()before the mock configuration for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/services/websocket/WebSocketProvider.test.tsx` around lines 81 - 86, Move the jest.clearAllMocks() call so it's executed before configuring the mock return to avoid confusion: call jest.clearAllMocks() first, then assign mockGetAuthJwt = authUtils.getAuthJwt as jest.MockedFunction<typeof authUtils.getAuthJwt> and then call mockGetAuthJwt.mockReturnValue("fresh-token"); this ensures clearAllMocks doesn't appear to affect the subsequent mock setup while preserving current test behavior.components/waves/marketplace/MarketplaceTransientNftPreview.tsx (1)
1-40: This component is identical toMarketplaceTransientMintPreview.Both components share the exact same logic and rendering flow. If no marketplace-specific divergence is planned, consider extracting a shared component (or factory) parameterized by name to reduce duplication. Fine to defer if per-marketplace customization is expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceTransientNftPreview.tsx` around lines 1 - 40, MarketplaceTransientNftPreview duplicates MarketplaceTransientMintPreview; extract the common logic and rendering into a shared component (e.g., MarketplaceTransientPreview or a factory) that accepts the same props (MarketplaceTypePreviewProps) and uses useMarketplacePreviewState, MarketplacePreviewPlaceholder, MarketplaceUnavailableCard, and MarketplaceItemPreviewCard for rendering; then have MarketplaceTransientNftPreview and MarketplaceTransientMintPreview become thin wrappers that call the shared component (or the factory) so behavior stays identical but duplication is removed.components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx (2)
78-91: Inline SVG duplicates the PlusIcon used elsewhere.
WaveleaderboardHeader.tsximportsPlusIconfrom@heroicons/react/24/solidfor the same button. Consider reusing that import here instead of inlining the SVG path.♻️ Reuse PlusIcon from heroicons
import React from "react"; import PrimaryButton from "@/components/utils/button/PrimaryButton"; +import { PlusIcon } from "@heroicons/react/24/solid"; ... <PrimaryButton loading={false} disabled={false} onClicked={onCreateDrop} padding="tw-px-4 tw-py-2" > - <svg - className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" - xmlns="http://www.w3.org/2000/svg" - viewBox="0 0 24 24" - fill="currentColor" - aria-hidden="true" - > - <path - fillRule="evenodd" - d="M12 3.75a.75.75 0 01.75.75v6.75h6.75a.75.75 0 010 1.5h-6.75v6.75a.75.75 0 01-1.5 0v-6.75H4.5a.75.75 0 010-1.5h6.75V4.5a.75.75 0 01.75-.75z" - clipRule="evenodd" - /> - </svg> + <PlusIcon className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" /> <span>Drop</span> </PrimaryButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx` around lines 78 - 91, WaveLeaderboardCurationEmptyState contains an inline SVG duplicate of the Plus icon; replace it by importing PlusIcon from `@heroicons/react/24/solid` and use <PlusIcon ... /> instead of the inline <svg> so styling/props remain consistent. Update the component WaveLeaderboardCurationEmptyState to add the import for PlusIcon and swap the inline SVG block with a PlusIcon element using the same className and aria-hidden attribute.
24-42: Hardcoded"Level 10"phrase creates fragile coupling with the eligibility message.The
levelPhraseconstant must exactly match the text produced bygetWaveDropEligibilityindropEligibility.ts(currently"Curation wave submissions require at least Level 10."). If that message changes, this emphasis logic silently degrades to rendering the plain string.Consider exporting the level threshold or phrase from a shared constant, or passing the structured data (e.g., level value) as a separate prop instead of parsing the rendered message string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/drops/WaveLeaderboardCurationEmptyState.tsx` around lines 24 - 42, The code in renderRestrictionMessage uses a hardcoded levelPhrase ("Level 10") which couples the UI to the exact text from getWaveDropEligibility; replace the hardcoded string by consuming a shared value instead: export a constant (e.g., LEVEL_PHRASE or LEVEL_THRESHOLD) from dropEligibility.ts or change the WaveLeaderboardCurationEmptyState component to accept a level/phrase prop, then use that imported/prop value in place of levelPhrase and in the split/emphasis logic (update the renderRestrictionMessage function and the levelPhrase reference accordingly) so the emphasis stays correct if the eligibility text changes.components/waves/CreateCurationDropContent.tsx (3)
113-143: Modal backdrop click handler usesstopPropagationon the outerdivbut relies on a separate backdrop<button>for closing.The
onClickon line 118 stops propagation for the overlay container, while line 124 has a full-screen button for closing. This is a reasonable pattern. However, the backdrop button has no visible focus indicator andtw-cursor-default, which may confuse keyboard users who tab into it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` around lines 113 - 143, The backdrop button used to close the modal (inside the modalContent element) is currently styled with tw-cursor-default and lacks visible focus indication, which harms keyboard accessibility; update the backdrop control (the full-screen <button> that uses onClick={onClose} and aria-label={`Close ${title}`}) to be clearly focusable and show a visible focus ring (remove tw-cursor-default, add a pointer cursor and appropriate focus-visible/focus:outline/focus:ring utility classes) so keyboard users can tab to it and see focus, keeping the button as the click target while leaving the outer div and modalRef/ModalLayout behaviors unchanged.
326-326:document.activeElementcan benull.
(document.activeElement as HTMLElement).blur()will throw ifactiveElementisnull. Although unlikely in this flow, a defensive check costs nothing.♻️ Defensive null check
- (document.activeElement as HTMLElement).blur(); + (document.activeElement as HTMLElement | null)?.blur();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` at line 326, In CreateCurationDropContent replace the unsafe cast and direct call to (document.activeElement as HTMLElement).blur() with a defensive check: ensure document.activeElement is non-null and an HTMLElement before calling blur (e.g., if (document.activeElement instanceof HTMLElement) document.activeElement.blur()), or use optional chaining with a proper type guard to avoid calling blur on null/unknown values.
79-107:onClosein the dependency array causes the effect to re-run on every parent render while the modal is open.The
onCloseprop is an inline arrow (() => setIsSupportedUrlsModalOpen(false)) on line 510, creating a new reference each render. WhileisOpenis true, every parent re-render will tear down and re-setup the effect — re-capturingpreviousActiveElementRef, toggling body overflow, and cycling the keydown listener.Wrap the close handler in
useCallbackat the call site, or removeonClosefrom the dependency array and read it via a ref inside the effect.♻️ Stabilize onClose via useCallback at the call site
In the parent component (
CreateCurationDropContent), around line 508-510:+ const closeSupportedUrlsModal = useCallback( + () => setIsSupportedUrlsModalOpen(false), + [] + ); ... <CurationInfoModal isOpen={isSupportedUrlsModalOpen} - onClose={() => setIsSupportedUrlsModalOpen(false)} + onClose={closeSupportedUrlsModal} title="Supported URLs" isApp={isApp} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` around lines 79 - 107, The effect that manages focus, body overflow and the Escape key is re-running because onClose is an unstable prop; stabilize it by either wrapping the parent close handler in useCallback (so the onClose passed into CreateCurationDropContent is referentially stable) or remove onClose from the useEffect dependency array and read a stable ref inside the effect (e.g., store onClose into onCloseRef.current before the effect and call onCloseRef.current() from the onKeyDown handler). Update references to onClose in the effect to use the stable callback or ref, keeping other deps (isApp, isOpen) intact and preserving previousActiveElementRef and modalRef behavior.__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx (1)
258-299: Scroll-fallback test uses a minimalAuthContextwithout a profile handle.Line 266 provides
connectedProfile: {}(nohandle), soisLoggedInevaluates tofalseand the drop button won't render. This is fine for the test's purpose (verifying scroll styling), but it's inconsistent with the other tests that explicitly provide{ handle: "tester" }. If this test is later extended to assert drop-button behavior, the missing handle will silently break expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/leaderboard/header/WaveleaderboardHeader.test.tsx` around lines 258 - 299, The test uses AuthContext.Provider with connectedProfile: {} which lacks a handle so isLoggedIn is false and the drop button won't render; update the test to provide a realistic connectedProfile (e.g., include handle: "tester") in the AuthContext.Provider used when rendering WaveLeaderboardHeader so the auth state matches other tests and future assertions about the drop button won't silently fail; ensure you only modify the connectedProfile object passed to AuthContext.Provider in this test (the rest of the props and the use of resolveControlModesMock/WaveLeaderboardHeader stay unchanged).components/waves/leaderboard/header/WaveleaderboardHeader.tsx (1)
272-288: Empty container rendered when logged in but ineligible to drop.When
isLoggedInis true butcanCreateDropis false (oronCreateDropis undefined), the outer<div>on line 273 still renders with classes and thetw-flexgap from the parent adds spacing against an empty element. This won't cause a visual break (flex items with no content collapse to zero width), but it's unnecessary DOM.♻️ Optional: avoid rendering the empty wrapper
- {isLoggedIn && ( - <div - className={`tw-flex tw-flex-col tw-items-end ${isMemesWave ? "lg:tw-hidden" : ""}`} - > - {canCreateDrop && onCreateDrop && ( - <PrimaryButton - loading={false} - disabled={false} - onClicked={onCreateDrop} - padding="tw-px-3 tw-py-2" - > - <PlusIcon className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" /> - <span>Drop</span> - </PrimaryButton> - )} - </div> - )} + {isLoggedIn && canCreateDrop && onCreateDrop && ( + <div + className={`tw-flex tw-flex-col tw-items-end ${isMemesWave ? "lg:tw-hidden" : ""}`} + > + <PrimaryButton + loading={false} + disabled={false} + onClicked={onCreateDrop} + padding="tw-px-3 tw-py-2" + > + <PlusIcon className="-tw-ml-1 tw-h-4 tw-w-4 tw-flex-shrink-0" /> + <span>Drop</span> + </PrimaryButton> + </div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/header/WaveleaderboardHeader.tsx` around lines 272 - 288, Render the wrapper only when there will be content: currently the outer div is shown whenever isLoggedIn is true even if canCreateDrop or onCreateDrop are false, producing an unnecessary empty element. Update the JSX in WaveleaderboardHeader so the outer <div> with classes "tw-flex tw-flex-col tw-items-end" is conditionally rendered together with the PrimaryButton (use the combined condition isLoggedIn && canCreateDrop && onCreateDrop) or move isLoggedIn into the inner condition that controls rendering of PrimaryButton/PlusIcon; ensure you keep the existing class conditional for isMemesWave.components/brain/my-stream/MyStreamWaveLeaderboard.tsx (2)
271-279: Persistent curation drop input always renders an emptyonSuccess.
onSuccess={() => {}}is a new anonymous function every render. SinceWaveDropCreatelikely accepts this as a prop, this could defeat memoization inside that component. Consider extracting a stable no-op callback.♻️ Stable no-op callback
Define outside the component or via
useCallback:+const NOOP = () => {}; + const MyStreamWaveLeaderboard: React.FC<MyStreamWaveLeaderboardProps> = ({Then use:
<WaveDropCreate wave={wave} - onSuccess={() => {}} + onSuccess={NOOP} isCurationLeaderboard />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveLeaderboard.tsx` around lines 271 - 279, The persistent drop input currently passes an anonymous inline onSuccess={() => {}} to WaveDropCreate which creates a new function each render and can break memoization; replace it with a stable no-op callback (either a module-level noop constant or a memoized function via useCallback) and pass that stable identifier as the onSuccess prop to WaveDropCreate when showPersistentDropInput is true so WaveDropCreate receives a referentially stable callback.
82-95:onCreateDropsilently no-ops for curation waves — verify this is communicated to children.When
isCurationWave && !isMemesWave, the callback body is a no-op. Lines 197 and 232–234 passundefinedin that case, so children won't render a trigger button. This is consistent, but worth a note: if a child ever receivesonCreateDropfor a curation wave (e.g., via a future code path), the click would silently do nothing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/MyStreamWaveLeaderboard.tsx` around lines 82 - 95, The onCreateDrop callback currently becomes a silent no-op when isCurationWave && !isMemesWave; change this so children get an explicit signal instead of undefined: keep the useCallback (onCreateDrop, mountedRef, isMemesWave, isCurationWave, setIsMemesCreateOpen, setIsCreateDropOpen) but when the curation-only case occurs return null (or undefined->null) and also add/pass an explicit prop like isCreateDisabled (boolean) or createDisabledReason so child components can render a disabled trigger or show a message; ensure callers/components that currently expect onCreateDrop check for null and/or the new isCreateDisabled prop and behave accordingly.components/waves/CreateDrop.tsx (1)
237-264:createDropContentPropsis computed even whenisCurationDropModeis true.When the curation path is active,
CreateDropContentis not rendered, butcreateDropContentPropsis still memoized every render cycle. This is harmless due touseMemo, but you could short-circuit the memo whenisCurationDropModeis true to avoid the allocation entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDrop.tsx` around lines 237 - 264, The memoized createDropContentProps is computed even when isCurationDropMode is true and CreateDropContent is never rendered; update the useMemo to short-circuit when isCurationDropMode is true (e.g., make the first line inside useMemo return undefined/null if isCurationDropMode) and add isCurationDropMode to the dependency array so no allocation happens while in curation mode; locate the createDropContentProps hook and the CreateDropContent usage to ensure the consumer handles the short-circuited value.components/waves/MarketplaceItemPreviewCard.tsx (2)
274-278: Misleadingdata-testidattributes reference "manifold" for a generic marketplace card.
data-testid="manifold-item-card"(Line 278),"manifold-item-media"(Line 290), and"manifold-item-price"(Line 353) are Manifold-specific names on a component that handles all supported marketplaces. Since this is a new file and tests are also new, consider renaming them to be marketplace-agnostic (e.g.,"marketplace-item-card").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/MarketplaceItemPreviewCard.tsx` around lines 274 - 278, The component MarketplaceItemPreviewCard uses Manifold-specific data-testid attributes ("manifold-item-card", "manifold-item-media", "manifold-item-price") which are misleading for a multi-marketplace component; update these attributes inside MarketplaceItemPreviewCard (where getMarketplaceContainerClass is used and where the media and price elements are rendered) to marketplace-agnostic names (e.g., "marketplace-item-card", "marketplace-item-media", "marketplace-item-price") and ensure any related tests or selectors are updated to use the new test IDs.
275-275:hideActionsis hardcoded totrueonLinkPreviewCardLayout.The component always passes
hideActionstoLinkPreviewCardLayoutastrue(suppressing the layout's built-in buttons) and provides its ownOverlayActionButtonscontrolled by the component'shideActionsprop (Line 359). This is intentional but the dual meaning ofhideActionsat two levels could confuse future readers. A brief comment would clarify intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/MarketplaceItemPreviewCard.tsx` at line 275, The LinkPreviewCardLayout call in MarketplaceItemPreviewCard intentionally passes hideActions={true} to suppress the layout's built-in action buttons because this component renders its own OverlayActionButtons controlled by the MarketplaceItemPreviewCard prop hideActions; add a concise inline comment next to the LinkPreviewCardLayout JSX (and optionally above the MarketplaceItemPreviewCard prop definition) explaining that hideActions is forced true for the layout to avoid duplicate buttons and that the component-level hideActions prop controls the custom OverlayActionButtons rendering.components/waves/CreateDropContent.tsx (2)
585-585: Unnecessary aliasavailableFiles.
const availableFiles = files;adds indirection without value. Consider usingfilesdirectly, or add a comment if this is a placeholder for future filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropContent.tsx` at line 585, Remove the unnecessary alias by replacing uses of availableFiles with files (or delete the declaration entirely) inside CreateDropContent; the line "const availableFiles = files;" should be removed unless you plan to implement filtering — if it's intended as a future placeholder, add a clarifying comment instead of the redundant assignment to make intent explicit.
432-432: Unnecessary aliasisStormModeActive.
isStormModeActiveis assignedisStormModewithout any transformation. This adds a layer of indirection for no benefit. If this is scaffolding for future logic (e.g., combining multiple conditions), a brief comment explaining intent would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateDropContent.tsx` at line 432, The local alias isStormModeActive is a redundant direct assignment of isStormMode; remove the unnecessary variable and update any references of isStormModeActive in CreateDropContent.tsx to use isStormMode directly (or, if the alias was intended as scaffolding for future combined logic, replace the assignment with a short comment explaining the intended future use and keep the alias). Ensure no other code relies on the alias name before deleting or replacing it.components/waves/marketplace/common.ts (2)
298-343:sanitizeOpenSeaOverlayMediapreserves all images when every candidate is blocked.At Line 329, when
!hasAnyNonBlockedCandidate, the original data is returned unchanged (keeping blocked overlay URLs). This is a deliberate "better to show something than nothing" strategy. The intent is sound but worth a brief inline comment explaining it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/common.ts` around lines 298 - 343, The function sanitizeOpenSeaOverlayMedia currently returns the original data when hasAnyNonBlockedCandidate is false, which preserves blocked OpenSea overlay URLs by design; add a brief inline comment at the hasAnyNonBlockedCandidate check inside sanitizeOpenSeaOverlayMedia explaining that this is intentional ("better to show something than nothing") and that we intentionally keep the original data when every candidate is blocked to avoid producing an empty/blank preview, referencing the hasAnyNonBlockedCandidate variable and the decision to return data unchanged.
127-145:toPickedMediadefaults to"image/*"for unknown extensions, which may misclassify video/audio.When
candidate.typeis absent and the URL extension is unrecognized (or missing), the fallback is"image/*". If a marketplace serves video content (e.g., MP4 NFTs) without atypehint, this would causeMediaDisplayto attempt image rendering. Consider whether a more generic fallback (or returningundefinedto signal "unknown") would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/common.ts` around lines 127 - 145, toPickedMedia currently falls back to "image/*" when candidate.type and inferMimeTypeFromUrl(url) are missing, which can misclassify video/audio; update to treat unknown mime types as unknown instead: in toPickedMedia, compute mimeType = asNonEmptyString(candidate.type) ?? inferMimeTypeFromUrl(url); if mimeType is undefined, return undefined (i.e., don't return a PickedMedia with a guessed "image/*"); reference the function toPickedMedia, types MediaCandidate and PickedMedia, and helper inferMimeTypeFromUrl/asNonEmptyString and adjust callers (e.g., MediaDisplay consumers) if they expect a non-null PickedMedia.components/waves/utils/validateCurationDropUrl.ts (2)
1-4: Consider exportingCurationUrlValidationResultfor consumer type safety.The type is defined locally but consumers of
validateCurationDropInputcan only infer the return type. Exporting it would help callers type intermediate variables without resorting toReturnType<...>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/validateCurationDropUrl.ts` around lines 1 - 4, Export the CurationUrlValidationResult type so callers can reference it directly; update the type declaration from a local-only type to an exported type (export type CurationUrlValidationResult = { error: true; helperText: string } | null) and ensure validateCurationDropInput's return signature aligns with this exported type so consumers can import and use CurationUrlValidationResult instead of relying on ReturnType<typeof validateCurationDropInput>.
150-152: Canonical URL retainswww.prefix when present.
toCanonicalHttpsUrlusesurl.hostnamedirectly, so an input likewww.superrare.com/artwork/...produceshttps://www.superrare.com/artwork/...even thoughgetAllowedDomainresolved tosuperrare.com(withoutwww.). If canonical IDs are later compared for equality, this mismatch could cause issues. Consider using the resolved allowed domain instead:♻️ Suggested fix
You could thread the resolved domain through to
toCanonicalHttpsUrl, or normalize in-place:-const toCanonicalHttpsUrl = (url: URL): string => { - return `https://${normalizeHostname(url.hostname)}${url.pathname}`; +const toCanonicalHttpsUrl = (url: URL, allowedDomain: string): string => { + return `https://${allowedDomain}${url.pathname}`; };This would require updating the call sites in
normalizeCurationDropInputandisAllowedCurationUrlto pass the allowed domain through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/utils/validateCurationDropUrl.ts` around lines 150 - 152, toCanonicalHttpsUrl currently builds the canonical URL using url.hostname which preserves a leading "www." and can mismatch the domain resolved by getAllowedDomain; change toCanonicalHttpsUrl to accept the resolved allowed domain (or otherwise run normalizeHostname/getAllowedDomain result) and use that domain instead of url.hostname, then update call sites normalizeCurationDropInput and isAllowedCurationUrl to pass the resolved domain through when calling toCanonicalHttpsUrl so canonical IDs are consistent with getAllowedDomain and normalizeHostname.__tests__/components/waves/marketplace/MarketplaceOpenseaItemPreview.test.tsx (1)
116-118: Fragile assertion onmock.calls[0]— may break on re-renders.After
waitForconfirms the mock was called with the expected props, the assertion on Line 117 accessesmock.calls[0][0].title. If the component re-renders before the data resolves (e.g., loading → success), the first call could be a different render. Consider usingmock.calls.at(-1)ortoHaveBeenLastCalledWithinstead for robustness.♻️ Suggested fix
- expect( - mockMarketplaceItemPreviewCard.mock.calls[0][0].title - ).toBeUndefined(); + expect( + mockMarketplaceItemPreviewCard.mock.calls.at(-1)?.[0].title + ).toBeUndefined();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/marketplace/MarketplaceOpenseaItemPreview.test.tsx` around lines 116 - 118, The test's assertion reads the first recorded call on mockMarketplaceItemPreviewCard (mock.calls[0][0].title), which is fragile across re-renders; update the assertion to inspect the last call instead (e.g., use mockMarketplaceItemPreviewCard.mock.calls.at(-1)[0].title) or use jest matchers like expect(mockMarketplaceItemPreviewCard).toHaveBeenLastCalledWith(...) after the existing waitFor so the assertion targets the final props passed when data resolves.__tests__/services/websocket/MarketplacePreviewWebSocketSync.test.tsx (1)
46-53: VerifyuseWebSocketMessagereturn type matches mock.The mock returns
{ isConnected: true }, but theMarketplacePreviewWebSocketSynccomponent doesn't use the return value. This is fine for now, but if the hook's return type changes, this mock could diverge silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/services/websocket/MarketplacePreviewWebSocketSync.test.tsx` around lines 46 - 53, Mocked useWebSocketMessage return value doesn't match the hook's real return shape and may drift; update mockedUseWebSocketMessage mock (the implementation that sets mediaLinkUpdatedCallback for WsMessageType.MEDIA_LINK_UPDATED) to return an object that matches the actual hook's return type (import or reference the hook's return type/interface used by MarketplacePreviewWebSocketSync) instead of just { isConnected: true } so the test mock remains type-safe and resilient to future hook changes.components/waves/marketplace/MarketplaceOpenseaItemPreview.tsx (1)
9-43: Significant code duplication across marketplace preview components.This component is nearly identical to
MarketplaceOpenseaAssetPreview,MarketplaceTransientNftPreview,MarketplaceTransientMintPreview,MarketplaceFoundationMintPreview,MarketplaceManifoldListingPreview, andMarketplaceSuperrareArtworkPreview. The only difference is themodeparameter passed touseMarketplacePreviewState. Consider extracting a shared component parameterized bymode:♻️ Proposed shared component
// marketplace/MarketplaceTypePreview.tsx export default function MarketplaceTypePreview({ href, compact = false, mode = "default", }: MarketplaceTypePreviewProps & { mode?: string }) { const state = useMarketplacePreviewState({ href, mode }); if (state.href !== href || state.type === "loading") { return <MarketplacePreviewPlaceholder href={href} compact={compact} />; } if (state.type === "error") { return <MarketplaceUnavailableCard href={href} compact={compact} />; } const media = state.resolvedMedia; if (media) { return ( <MarketplaceItemPreviewCard href={href} mediaUrl={media.url} mediaMimeType={media.mimeType} price={state.resolvedPrice} title={state.resolvedTitle} compact={compact} hideActions={compact} /> ); } return <MarketplaceUnavailableCard href={href} compact={compact} />; }Then each specialized component becomes a thin wrapper or is replaced entirely in the switch statement in
MarketplacePreview.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceOpenseaItemPreview.tsx` around lines 9 - 43, There’s duplicated logic across MarketplaceOpenseaItemPreview and several other preview components; extract a single parameterized component (e.g., MarketplaceTypePreview) that accepts href, compact, and mode and encapsulates the shared flow using useMarketplacePreviewState({ href, mode }); render MarketplacePreviewPlaceholder when state.href !== href || state.type === "loading", MarketplaceUnavailableCard when state.type === "error" or no resolvedMedia, and MarketplaceItemPreviewCard when media exists (passing media.url, media.mimeType, state.resolvedPrice, state.resolvedTitle, compact, hideActions). Replace each specialized component (MarketplaceOpenseaItemPreview, MarketplaceOpenseaAssetPreview, MarketplaceTransientNftPreview, MarketplaceTransientMintPreview, MarketplaceFoundationMintPreview, MarketplaceManifoldListingPreview, MarketplaceSuperrareArtworkPreview) with a thin wrapper that calls MarketplaceTypePreview with the appropriate mode or update the switch in MarketplacePreview.tsx to use MarketplaceTypePreview directly.__tests__/components/waves/marketplace/urlKind.test.ts (1)
44-45: Consider documenting or testing multi-chain support expectations.Line 45 confirms that
opensea.io/item/base/...is rejected — only Ethereum chain paths are recognized. If this is intentional, it might be worth a brief comment in the URL kind implementation explaining why other chains are excluded, to prevent future contributors from treating this as a bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/marketplace/urlKind.test.ts` around lines 44 - 45, The test shows opensea URLs with non-Ethereum chains (e.g., "base") are rejected; if that is intentional, add a short clarifying comment in the URL-kind implementation (e.g., inside urlKind / parseOpenSeaUrl / isOpenSeaAssetUrl where OpenSea path parsing happens) stating that only Ethereum chain paths are recognized and why other chains are excluded, and optionally add/update a unit test that asserts non-EVM/Ethereum chain segments (like "base") are rejected to document the expectation.__tests__/components/waves/marketplace/MarketplaceOpenseaAssetPreview.test.tsx (1)
115-127: Consider documenting the implicit MIME-type inference from URL extension.
mediaMimeType: "image/png"is derived from the.pngextension inmedia_uriby the component, since the NFT-link API response contains no explicit MIME field. A brief inline comment would make this intent self-documenting and clarify why changing the test URL to an extension-less CDN path would break the assertion.✏️ Suggested comment
await waitFor(() => expect(mockMarketplaceItemPreviewCard).toHaveBeenCalledWith( expect.objectContaining({ href, mediaUrl: "https://cdn.example.com/nft-image.png", - mediaMimeType: "image/png", + mediaMimeType: "image/png", // inferred from .png extension; no MIME in nft-link response price: "0.5 ETH", title: "OpenSea Asset `#42`", }) ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/marketplace/MarketplaceOpenseaAssetPreview.test.tsx` around lines 115 - 127, The test asserts mediaMimeType: "image/png" but the MIME type is implicitly inferred from the media_uri file extension by the component; add a brief inline comment in MarketplaceOpenseaAssetPreview.test.tsx near the mock data and the expect for mockMarketplaceItemPreviewCard explaining that mediaMimeType is derived from the .png extension in media_uri (since the NFT-link API response lacks an explicit MIME field) so changing the test URL to an extension-less CDN path would break the assertion; reference the mockMarketplaceItemPreviewCard assertion and the media_uri field in the test to make the linkage clear.helpers/Types.tsx (2)
362-375: Add aWsMediaLinkUpdatedMessagewrapper interface to match the pattern for other WS types.Both
WsTypingMessageandWsDropUpdateMessagepair atypediscriminant with adatapayload, forming a discriminated union that callers can switch on.WsMediaLinkUpdatedDataexports only the data shape, leaving theMEDIA_LINK_UPDATEDenum member without a corresponding message type. Consumers (e.g.,MarketplacePreviewWebSocketSync.tsx) must manually inline or unsafely cast the envelope, losing the type-safety guarantees the pattern provides.♻️ Proposed addition
export interface WsMediaLinkUpdatedData { readonly canonical_id: string; readonly platform: string | null; // ... readonly failed_since: string | number | null; } + +export interface WsMediaLinkUpdatedMessage { + readonly type: WsMessageType.MEDIA_LINK_UPDATED; + readonly data: WsMediaLinkUpdatedData; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helpers/Types.tsx` around lines 362 - 375, Add a discriminated envelope interface for the MEDIA_LINK_UPDATED websocket event to match the existing pattern: define WsMediaLinkUpdatedMessage with a readonly type property set to the MEDIA_LINK_UPDATED enum member and a readonly data property of type WsMediaLinkUpdatedData; update exports so callers (e.g., MarketplacePreviewWebSocketSync.tsx) can consume the discriminated union the same way they do WsTypingMessage and WsDropUpdateMessage without unsafe casts.
373-374: Consider narrowingstring | number | nulltimestamp fields.
last_successfully_updatedandfailed_sinceallow bothstringandnumber, which forces every consumer to branch ontypeof. If the backend exclusively returns Unix epoch integers, tighten tonumber | null; if it can return ISO 8601 strings, document this explicitly so consumers know which to expect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helpers/Types.tsx` around lines 373 - 374, Confirm the backend timestamp format and then either tighten the two fields or document them: if the API returns Unix epoch integers, change the types of last_successfully_updated and failed_since from "string | number | null" to "number | null"; if the API can return ISO 8601 strings, add clear JSDoc comments above the last_successfully_updated and failed_since properties stating the exact expected format (e.g., "ISO 8601 string" or "Unix epoch ms integer") so consumers know which type to expect.components/waves/marketplace/urlKind.ts (1)
90-109: Approved — consider a one-line comment on the intentional early return.The logic is correct: once a matching domain is found and no path pattern matches,
return nullat line 105 prevents fall-through to other entries in the registry (which can never match the same hostname). Because this early exit looks identical to a missingcontinue, a brief comment keeps the intent clear for future maintainers who might add a second entry for the same host.✏️ Optional comment
for (const { pattern, kind } of matchers) { if (pattern.test(pathname)) { return kind; } } - return null; + // Domain matched but no path pattern did — stop searching (no other entry owns this host). + return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/urlKind.ts` around lines 90 - 109, Add a one-line comment in getMarketplaceKindForHostAndPath (the function that iterates MARKETPLACE_HOST_MATCHERS and uses isApexOrWwwHost) directly before the early `return null` inside the outer loop to explain that this return intentionally exits after finding the matching domain when no path pattern matched (preventing fall-through to other registry entries for the same hostname); keep the comment brief and placed next to the inner loop so future maintainers understand the early exit is deliberate.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/websocket/WebSocketProvider.tsx (1)
73-74:⚠️ Potential issue | 🟡 MinorComment/code mismatch in backoff formula.
The comment says
2^attemptbut the code uses1.5as the base. Trivial, but worth keeping in sync.- // Exponential backoff formula: initialDelay * 2^attempt (capped at maxDelay) - const delay = initialDelay * Math.pow(1.5, attempt); + // Exponential backoff formula: initialDelay * 1.5^attempt (capped at maxDelay) + const delay = initialDelay * Math.pow(1.5, attempt);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/WebSocketProvider.tsx` around lines 73 - 74, The comment describing the exponential backoff is out of sync with the implementation: the code computes delay using base 1.5 (delay = initialDelay * Math.pow(1.5, attempt)) while the comment says 2^attempt; update them to match by either changing the comment to state "initialDelay * 1.5^attempt (capped at maxDelay)" or changing the code to use Math.pow(2, attempt) depending on desired backoff behavior; adjust the comment near the delay calculation (variables: delay, initialDelay, attempt, maxDelay in WebSocketProvider component) so the comment and implementation are consistent.
🧹 Nitpick comments (6)
components/waves/marketplace/common.ts (1)
34-37: Consider exportingPickedMedia.
PickedMediais referenced in the exportedMarketplacePreviewStateandMarketplacePreviewDatatypes. While TypeScript's structural typing lets consumers work without importing it directly, exporting the type improves DX by allowing explicit type annotations in consuming code.-type PickedMedia = { +export type PickedMedia = { readonly url: string; readonly mimeType: string; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/common.ts` around lines 34 - 37, Export the PickedMedia type so consumers can import it explicitly: update the declaration of PickedMedia to be exported (export type PickedMedia = { url: string; mimeType: string; };) since it is referenced by the exported MarketplacePreviewState and MarketplacePreviewData types (ensure the name PickedMedia remains unchanged so existing references in MarketplacePreviewState and MarketplacePreviewData continue to resolve).__tests__/components/waves/marketplace/MarketplaceManifoldListingPreview.test.tsx (1)
121-155: Fragile:fetchLinkPreviewis not mocked in this test but is expected to be called.
fetchLinkPreviewdefaults to a barejest.fn()(returnsundefined). The hook callsawait fetchLinkPreview(href), which yieldsundefined, thenmergeOpenGraphFallbackaccesses properties on it and throws aTypeError. The hook'scatchblock happens to return the nft-link preview becausepreview.media !== null, so the test passes — but through an unintended error path.Consider adding an explicit mock to make the intent clear:
Suggested fix
+ fetchLinkPreview.mockResolvedValue({ + title: "Fallback Title", + }); + renderWithQueryClient(<MarketplaceManifoldListingPreview href={href} />);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/marketplace/MarketplaceManifoldListingPreview.test.tsx` around lines 121 - 155, The test is fragile because fetchLinkPreview is a bare jest.fn() and returns undefined causing mergeOpenGraphFallback to throw; explicitly mock fetchLinkPreview in the test (the one used by MarketplaceManifoldListingPreview) to return a safe value (e.g. null or an object with expected OG fields) so the hook's await fetchLinkPreview(href) doesn't produce undefined, then assert fetchLinkPreview was called; update the test that currently sets fetchNftLink and calls renderWithQueryClient and mockMarketplaceItemPreviewCard to include this explicit mock of fetchLinkPreview to make the intent and control flow deterministic.services/websocket/WebSocketProvider.tsx (1)
43-63: Resolve outer/inner message type-data coupling to prevent schema mismatches.The function can mix outer
typewith innerdata(lines 52–61). Whenpayload.typeexists butpayload.datais absent, the type comes from the outer envelope while data falls through tonestedPayload?.data. If the server ever sends messages where the outer and inner types differ, subscribers would receive data that doesn't match their expected schema.The nested message structure is explicitly part of
WebSocketMessagePayload, so this fallback behavior is intentional. However, it's fragile and relies on undocumented guarantees. Resolving bothtypeanddatafrom the same level (outer first, then nested) would make the contract explicit and safer:Suggested approach
const normalizeIncomingMessage = ( value: unknown ): { readonly type: string; readonly data: unknown } | undefined => { const payload = asWebSocketMessagePayload(value); if (!payload) { return undefined; } - const nestedPayload = asWebSocketMessagePayload(payload.message); - const type = - asNonEmptyString(payload.type) ?? asNonEmptyString(nestedPayload?.type); - - if (!type) { - return undefined; + const outerType = asNonEmptyString(payload.type); + if (outerType) { + return { type: outerType, data: payload.data }; } - return { - type, - data: payload.data ?? nestedPayload?.data, - }; + const nestedPayload = asWebSocketMessagePayload(payload.message); + const nestedType = asNonEmptyString(nestedPayload?.type); + if (nestedType) { + return { type: nestedType, data: nestedPayload?.data }; + } + + return undefined; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/websocket/WebSocketProvider.tsx` around lines 43 - 63, normalizeIncomingMessage currently mixes outer type with inner data which can cause schema mismatches; change the selection logic so both type and data are taken from the same envelope: if asNonEmptyString(payload.type) is present, treat payload as the source (use payload.type and payload.data, even if data is undefined), otherwise if nestedPayload has a type use nestedPayload's type and data, otherwise return undefined. Update references to payload/nestedPayload accordingly in normalizeIncomingMessage so type and data are always derived from the same object.components/waves/CreateCurationDropContent.tsx (3)
79-107:onClosereference instability causes spurious focus cleanup while modal is open.
onCloseis declared inline at the call site (() => setIsSupportedUrlsModalOpen(false)) and included in theuseEffectdependency array. Every parent re-render while the modal is open produces a newonClosereference, which triggers the cleanup path — includingpreviousActiveElementRef.current?.focus()— pulling focus away from the open modal before the effect re-establishes it.♻️ Proposed fix — stabilize `onClose` via a ref inside the effect
+ const onCloseRef = useRef(onClose); + useLayoutEffect(() => { onCloseRef.current = onClose; }, [onClose]); useEffect(() => { if (!isOpen) { return; } previousActiveElementRef.current = document.activeElement as HTMLElement; modalRef.current?.focus(); const originalOverflow = document.body.style.overflow; if (!isApp) { document.body.style.overflow = "hidden"; } const onKeyDown = (event: KeyboardEvent) => { if (event.key === "Escape") { - onClose(); + onCloseRef.current(); } }; document.addEventListener("keydown", onKeyDown); return () => { if (!isApp) { document.body.style.overflow = originalOverflow; } document.removeEventListener("keydown", onKeyDown); previousActiveElementRef.current?.focus(); }; - }, [isApp, isOpen, onClose]); + }, [isApp, isOpen]);Alternatively, wrap
onClosewithuseCallbackat the call site inCreateCurationDropContent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` around lines 79 - 107, The effect's dependency on an inline onClose causes cleanup to run when the parent re-renders; fix by stabilizing the onClose reference used inside the useEffect: capture the current onClose into a ref at the top of the effect (e.g., const onCloseRef = useRef(onClose); onCloseRef.current = onClose) and use onCloseRef.current inside the onKeyDown handler and cleanup, or alternatively require the parent to pass a memoized onClose via useCallback; ensure references to previousActiveElementRef, modalRef and document body overflow logic remain unchanged.
531-531:<TermsSignatureFlow />is unconditionally mounted regardless of signature requirements.The component is dynamically imported for lazy loading, but it is rendered on every mount even when
wave.participation.signature_requiredisfalse. This eagerly triggers the chunk download and mounts the component unnecessarily for non-signature waves.♻️ Proposed fix
- <TermsSignatureFlow /> + {wave.participation.signature_required && <TermsSignatureFlow />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` at line 531, The TermsSignatureFlow component is being mounted unconditionally which causes its dynamic chunk to load even when not needed; update CreateCurationDropContent to only render (and thus only load) TermsSignatureFlow when wave.participation.signature_required is true by wrapping the render in a conditional check (e.g., use wave.participation.signature_required && <TermsSignatureFlow />) so the dynamic import for TermsSignatureFlow is not triggered for non-signature waves; keep any existing Suspense/dynamic import setup but move the conditional to the JSX that currently renders <TermsSignatureFlow />.
113-135:CurationInfoModallacks a focus trap — Tab key can escape the modal.The component initializes focus via
modalRef.current?.focus()and handles Escape key closure, but does not trap Tab/Shift+Tab navigation within the modal boundary. Users can Tab out of the modal while it's open, violating the ARIA Authoring Practices Guide for modal dialogs.The
<dialog>element here uses the declarativeopenattribute (rather thandialog.showModal()) and relies on custom styling and portals for positioning, so the browser's built-in focus containment does not apply.Implement a focus trap to keep Tab/Shift+Tab navigation within focusable descendants of the modal (via
focus-trap-reactor a custom hook).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` around lines 113 - 135, The modal currently sets initial focus via modalRef and handles Escape but doesn’t trap Tab/Shift+Tab; update the CreateCurationDropContent modal to install a focus trap around the dialog content (use focus-trap-react or a small custom hook) so keyboard Tab/Shift+Tab cycles only among the modal’s focusable descendants; apply the trap to the same element referenced by modalRef (or wrap ModalLayout) and ensure the trap activates when open and deactivates/returns focus when onClose is called (symbols to modify: modalRef, modalContent / the <dialog> wrapper, ModalLayout, and onClose).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/waves/CreateCurationDropContent.tsx`:
- Around line 313-315: The call to addOptimisticDrop inside the optimisticDrop
branch currently uses "void addOptimisticDrop({ drop: optimisticDrop })" which
swallows rejections and can leave stale optimistic state; replace it with a
handled promise (either await the call inside an async function or attach
.catch) and, on error, call the existing toast/error handler and revert any
optimistic state (e.g., remove the optimistic drop) so failures surface to the
user; update the code around optimisticDrop, addOptimisticDrop, and the
surrounding setTimeout cleanup to ensure errors are caught and handled.
---
Outside diff comments:
In `@services/websocket/WebSocketProvider.tsx`:
- Around line 73-74: The comment describing the exponential backoff is out of
sync with the implementation: the code computes delay using base 1.5 (delay =
initialDelay * Math.pow(1.5, attempt)) while the comment says 2^attempt; update
them to match by either changing the comment to state "initialDelay *
1.5^attempt (capped at maxDelay)" or changing the code to use Math.pow(2,
attempt) depending on desired backoff behavior; adjust the comment near the
delay calculation (variables: delay, initialDelay, attempt, maxDelay in
WebSocketProvider component) so the comment and implementation are consistent.
---
Duplicate comments:
In `@app/api/open-graph/foundation/service.ts`:
- Line 5: You consolidated the helper by importing asNonEmptyString but there’s
a duplicate helper exported elsewhere; pick a single source-of-truth: remove the
duplicate implementation and have the other module re-export asNonEmptyString
(or vice-versa) so all callers import the same symbol; update references to use
the unified export (asNonEmptyString) and add a short re-export in the other
module to preserve backward compatibility while eliminating duplicate logic.
In `@components/waves/CreateCurationDropContent.tsx`:
- Around line 216-240: The Promise returned where you dispatch the
"showTermsModal" CustomEvent can hang if TermsSignatureFlow hasn't mounted;
update the logic in CreateCurationDropContent (the Promise that creates
handleSigningComplete and dispatches the event) to add a readiness guard and
timeout: attach a temporary event listener or check a new "termsModalReady"
signal before dispatching, and also start a clearable timeout (e.g., 10s) that
resolves the Promise with null if no response arrives; ensure you remove/cleanup
the timeout and any listeners when handleSigningComplete runs so the Promise
always settles even if TermsSignatureFlow never mounts.
In `@components/waves/marketplace/common.ts`:
- Around line 1-6: The import of asNonEmptyString should be unified to the
centralized helper path; update the server-side files that still import the
duplicated helper (foundation/service.ts and manifold/service.ts) to import
asNonEmptyString from "@/lib/text/nonEmptyString" instead of "../opensea/shared"
(or any local copy), so both client- and server-side code use the same helper
symbol asNonEmptyString and avoid duplication.
---
Nitpick comments:
In
`@__tests__/components/waves/marketplace/MarketplaceManifoldListingPreview.test.tsx`:
- Around line 121-155: The test is fragile because fetchLinkPreview is a bare
jest.fn() and returns undefined causing mergeOpenGraphFallback to throw;
explicitly mock fetchLinkPreview in the test (the one used by
MarketplaceManifoldListingPreview) to return a safe value (e.g. null or an
object with expected OG fields) so the hook's await fetchLinkPreview(href)
doesn't produce undefined, then assert fetchLinkPreview was called; update the
test that currently sets fetchNftLink and calls renderWithQueryClient and
mockMarketplaceItemPreviewCard to include this explicit mock of fetchLinkPreview
to make the intent and control flow deterministic.
In `@components/waves/CreateCurationDropContent.tsx`:
- Around line 79-107: The effect's dependency on an inline onClose causes
cleanup to run when the parent re-renders; fix by stabilizing the onClose
reference used inside the useEffect: capture the current onClose into a ref at
the top of the effect (e.g., const onCloseRef = useRef(onClose);
onCloseRef.current = onClose) and use onCloseRef.current inside the onKeyDown
handler and cleanup, or alternatively require the parent to pass a memoized
onClose via useCallback; ensure references to previousActiveElementRef, modalRef
and document body overflow logic remain unchanged.
- Line 531: The TermsSignatureFlow component is being mounted unconditionally
which causes its dynamic chunk to load even when not needed; update
CreateCurationDropContent to only render (and thus only load) TermsSignatureFlow
when wave.participation.signature_required is true by wrapping the render in a
conditional check (e.g., use wave.participation.signature_required &&
<TermsSignatureFlow />) so the dynamic import for TermsSignatureFlow is not
triggered for non-signature waves; keep any existing Suspense/dynamic import
setup but move the conditional to the JSX that currently renders
<TermsSignatureFlow />.
- Around line 113-135: The modal currently sets initial focus via modalRef and
handles Escape but doesn’t trap Tab/Shift+Tab; update the
CreateCurationDropContent modal to install a focus trap around the dialog
content (use focus-trap-react or a small custom hook) so keyboard Tab/Shift+Tab
cycles only among the modal’s focusable descendants; apply the trap to the same
element referenced by modalRef (or wrap ModalLayout) and ensure the trap
activates when open and deactivates/returns focus when onClose is called
(symbols to modify: modalRef, modalContent / the <dialog> wrapper, ModalLayout,
and onClose).
In `@components/waves/marketplace/common.ts`:
- Around line 34-37: Export the PickedMedia type so consumers can import it
explicitly: update the declaration of PickedMedia to be exported (export type
PickedMedia = { url: string; mimeType: string; };) since it is referenced by the
exported MarketplacePreviewState and MarketplacePreviewData types (ensure the
name PickedMedia remains unchanged so existing references in
MarketplacePreviewState and MarketplacePreviewData continue to resolve).
In `@services/websocket/WebSocketProvider.tsx`:
- Around line 43-63: normalizeIncomingMessage currently mixes outer type with
inner data which can cause schema mismatches; change the selection logic so both
type and data are taken from the same envelope: if
asNonEmptyString(payload.type) is present, treat payload as the source (use
payload.type and payload.data, even if data is undefined), otherwise if
nestedPayload has a type use nestedPayload's type and data, otherwise return
undefined. Update references to payload/nestedPayload accordingly in
normalizeIncomingMessage so type and data are always derived from the same
object.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx (2)
129-139: All three branches of thecardClassNameIIFE return an identical string — the conditional structure is dead code.
isCompactMode,isMarketplaceImageOnlyCard, and the default case all produce the same Tailwind class string. The IIFE can be collapsed to a single constant, removing the misleading branching.♻️ Proposed simplification
- const cardClassName = (() => { - if (isCompactMode) { - return "tw-cursor-pointer tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-iron-800 tw-bg-iron-950 tw-p-0 tw-transition desktop-hover:hover:tw-border-iron-700"; - } - - if (isMarketplaceImageOnlyCard) { - return "tw-cursor-pointer tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-iron-800 tw-bg-iron-950 tw-p-0 tw-transition desktop-hover:hover:tw-border-iron-700"; - } - - return "tw-cursor-pointer tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-iron-800 tw-bg-iron-950 tw-p-0 tw-transition desktop-hover:hover:tw-border-iron-700"; - })(); + const cardClassName = + "tw-cursor-pointer tw-overflow-hidden tw-rounded-xl tw-border tw-border-solid tw-border-iron-800 tw-bg-iron-950 tw-p-0 tw-transition desktop-hover:hover:tw-border-iron-700";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx` around lines 129 - 139, The IIFE for cardClassName contains dead conditional branches (isCompactMode, isMarketplaceImageOnlyCard, and the default all return the same Tailwind string); replace the IIFE with a single constant assignment for cardClassName using that common class string and remove the unused conditional checks/branches to simplify the component (refer to cardClassName, isCompactMode, and isMarketplaceImageOnlyCard to locate and remove the redundant logic).
309-336:<LinkPreviewProvider>+<WaveDropPartContentMarkdown>duplicated verbatim across both branches — only the wrapper div differs.Both the
shouldPadContentOnlyTexttrue- and false-paths render the same component tree with the same props. Extract the shared subtree and conditionally apply the padding wrapper:♻️ Proposed refactor
- {activePart && - (shouldPadContentOnlyText ? ( - <div className="tw-p-2"> - <LinkPreviewProvider variant="home"> - <WaveDropPartContentMarkdown - mentionedUsers={drop.mentioned_users} - mentionedWaves={drop.mentioned_waves} - referencedNfts={drop.referenced_nfts} - part={activePart} - wave={drop.wave} - drop={drop} - onQuoteClick={() => {}} - /> - </LinkPreviewProvider> - </div> - ) : ( - <LinkPreviewProvider variant="home"> - <WaveDropPartContentMarkdown - mentionedUsers={drop.mentioned_users} - mentionedWaves={drop.mentioned_waves} - referencedNfts={drop.referenced_nfts} - part={activePart} - wave={drop.wave} - drop={drop} - onQuoteClick={() => {}} - /> - </LinkPreviewProvider> - ))} + {activePart && ( + <LinkPreviewProvider variant="home"> + <div className={shouldPadContentOnlyText ? "tw-p-2" : undefined}> + <WaveDropPartContentMarkdown + mentionedUsers={drop.mentioned_users} + mentionedWaves={drop.mentioned_waves} + referencedNfts={drop.referenced_nfts} + part={activePart} + wave={drop.wave} + drop={drop} + onQuoteClick={() => {}} + /> + </div> + </LinkPreviewProvider> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx` around lines 309 - 336, The rendering duplicates the same LinkPreviewProvider + WaveDropPartContentMarkdown subtree in both branches of the shouldPadContentOnlyText conditional; refactor by computing the shared subtree (the LinkPreviewProvider wrapping WaveDropPartContentMarkdown with props mentionedUsers={drop.mentioned_users}, mentionedWaves={drop.mentioned_waves}, referencedNfts={drop.referenced_nfts}, part={activePart}, wave={drop.wave}, drop={drop}, onQuoteClick={() => {}}) once and then conditionally wrap that subtree with a div className="tw-p-2" when shouldPadContentOnlyText is true, leaving the existing conditional rendering of activePart unchanged.__tests__/components/waves/marketplace/common.test.ts (1)
8-9: Export and import the query-key helper to keep tests in sync with production.
getMarketplacePreviewQueryKeyis currently duplicated in the test file. The production module constructs an identical key at line 276 ([MARKETPLACE_PREVIEW_QUERY_KEY, { href, mode }]) but does not export this helper or the constant. Export both from productioncommon.tsand import them here to avoid brittle, out-of-sync test implementations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/marketplace/common.test.ts` around lines 8 - 9, Export the query-key constant and helper from the production module and import them into the test to eliminate the duplicated implementation: add exports for MARKETING_PREVIEW_QUERY_KEY (named in production as MARKETPLACE_PREVIEW_QUERY_KEY) and the helper getMarketplacePreviewQueryKey in the production common.ts (the same symbols used to build [MARKETPLACE_PREVIEW_QUERY_KEY, { href, mode }]) and update __tests__/components/waves/marketplace/common.test.ts to import and use those exports instead of its local const; ensure the exported helper returns the exact tuple shape used in production so tests remain in sync.components/waves/marketplace/MarketplaceItemPreviewMediaLink.tsx (1)
28-31:data-testid="manifold-item-media"is Manifold-specific but used for all marketplacesThe test ID on the inner media wrapper is named after Manifold, yet this component is composed by all marketplace previews (OpenSea, Foundation, SuperRare, Transient, etc.). Consider renaming to
"marketplace-item-media"for consistency with"marketplace-item-media-link"on the outerLink.♻️ Rename data-testid to generic name
- data-testid="manifold-item-media" + data-testid="marketplace-item-media"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceItemPreviewMediaLink.tsx` around lines 28 - 31, The inner media wrapper in MarketplaceItemPreviewMediaLink uses a Manifold-specific test ID ("manifold-item-media") but this component serves multiple marketplaces; change the data-testid on the div with class `${MARKETPLACE_MEDIA_FRAME_CLASS} tw-relative` inside the MarketplaceItemPreviewMediaLink component to "marketplace-item-media" so it matches the outer Link's "marketplace-item-media-link" naming and is marketplace-agnostic.components/waves/marketplace/MarketplaceCopyListingButton.tsx (1)
16-29:setTimeoutnot cleaned up on unmount and accumulates on rapid clicksIf the component unmounts within the 500 ms window (e.g., a parent modal closes), the timeout still fires and calls
setCopied(false). Additionally, multiple rapid clicks schedule multiple concurrent timeouts. Both issues are addressed by tracking the timer in auseRefand clearing it in auseEffectcleanup.♻️ Proposed fix using useRef + useEffect
-import { useState, type MouseEvent } from "react"; +import { useEffect, useRef, useState, type MouseEvent } from "react"; import { stopPropagation } from "./MarketplaceItemPreviewCard.utils"; ... export default function MarketplaceCopyListingButton(...) { const [copied, setCopied] = useState(false); + const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + + useEffect(() => () => { + if (timeoutRef.current !== null) clearTimeout(timeoutRef.current); + }, []); const copyToClipboard = (event: MouseEvent<HTMLButtonElement>) => { stopPropagation(event); void navigator.clipboard .writeText(href) .then(() => { setCopied(true); - setTimeout(() => setCopied(false), 500); + if (timeoutRef.current !== null) clearTimeout(timeoutRef.current); + timeoutRef.current = setTimeout(() => { + timeoutRef.current = null; + setCopied(false); + }, 500); }) .catch(() => { // Ignore clipboard write failures (e.g. missing permissions). }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceCopyListingButton.tsx` around lines 16 - 29, The copyToClipboard handler currently uses setTimeout without cleanup causing stale updates and multiple timers; change this by creating a timerRef via useRef<number | null>(null), replace setTimeout calls with assigning window.setTimeout(...) to timerRef.current, clear any existing timer before setting a new one, and add a useEffect cleanup that clears timerRef.current on unmount; update references to setCopied and copied as before and keep stopPropagation(event)/navigator.clipboard.writeText(href) logic intact in copyToClipboard.components/waves/marketplace/MarketplacePreviewPlaceholder.tsx (1)
15-18: Redundantvariantprop —LinkPreviewCardLayoutalready reads the same context
useLinkPreviewVariant()is called here at line 15, then immediately forwarded asvariant={variant}toLinkPreviewCardLayout. However,LinkPreviewCardLayoutitself callsuseLinkPreviewVariant()internally and falls back to context when novariantprop is supplied (const resolvedVariant = variant ?? contextVariant). The explicit forward is a no-op.♻️ Remove redundant hook call and prop
- const variant = useLinkPreviewVariant(); - return ( - <LinkPreviewCardLayout href={href} variant={variant} hideActions> + <LinkPreviewCardLayout href={href} hideActions> <div - className={`${getMarketplaceContainerClass(variant, compact)} tw-relative`} + className={`${getMarketplaceContainerClass(useLinkPreviewVariant(), compact)} tw-relative`}Or keep the hook call for the class computation (which legitimately needs the variant) and simply drop it from the
LinkPreviewCardLayoutprop:const variant = useLinkPreviewVariant(); return ( - <LinkPreviewCardLayout href={href} variant={variant} hideActions> + <LinkPreviewCardLayout href={href} hideActions>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplacePreviewPlaceholder.tsx` around lines 15 - 18, The explicit variant prop forwarded to LinkPreviewCardLayout is redundant because LinkPreviewCardLayout already calls useLinkPreviewVariant() and falls back to context; remove the prop forwarding (drop variant={variant} from the LinkPreviewCardLayout JSX in MarketplacePreviewPlaceholder) and keep or remove the local useLinkPreviewVariant() call depending on usage—if MarketplacePreviewPlaceholder uses variant for className or other local computation, keep the hook and only remove the prop; if not, delete the hook call entirely. Ensure references to useLinkPreviewVariant and LinkPreviewCardLayout (and the component MarketplacePreviewPlaceholder) are updated accordingly.components/waves/marketplace/MarketplaceUnavailableCard.tsx (1)
19-24: Duplicated href-resolution logic — consider reusingresolvePreviewHref.Lines 19–24 replicate the same pattern as
resolvePreviewHrefinMarketplaceItemPreviewCard.utils.ts(lines 15–26): strip base endpoint, check for leading/, branch on relative vs external. Using the shared utility keeps the two card components consistent and removes a maintenance surface.♻️ Suggested refactor
import { removeBaseEndpoint } from "@/helpers/Helpers"; +import { resolvePreviewHref } from "./MarketplaceItemPreviewCard.utils"; import { useLinkPreviewVariant } from "../LinkPreviewContext"; ... export default function MarketplaceUnavailableCard({ href, compact = false, }: MarketplaceTypePreviewProps) { const variant = useLinkPreviewVariant(); - const relative = removeBaseEndpoint(href); - const isRelativeHref = - typeof relative === "string" && relative.startsWith("/"); - const resolvedHref = isRelativeHref ? relative : href; - const target = isRelativeHref ? undefined : "_blank"; - const rel = isRelativeHref ? undefined : "noopener noreferrer"; + const { href: resolvedHref, target, rel } = resolvePreviewHref(href);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceUnavailableCard.tsx` around lines 19 - 24, The href resolution logic in MarketplaceUnavailableCard (the block computing relative, isRelativeHref, resolvedHref, target, rel) duplicates resolvePreviewHref from MarketplaceItemPreviewCard.utils.ts; replace the manual logic by importing and calling resolvePreviewHref to get the resolved href and target/rel behavior (or adapt its return to derive target/rel) so both components share the same implementation and eliminate duplication.components/waves/marketplace/common.ts (1)
373-418: Intentional but subtle: when all candidates are blocked, no sanitization occurs.In
sanitizeOpenSeaOverlayMedia, if every image candidate (primary +imagesarray) is an OpenSea overlay URL,hasAnyNonBlockedCandidateisfalseand the originaldatais returned unchanged (line 405). This means blocked overlay images are preserved as a last resort. The behavior is correct as a safety net (avoids stripping all media), but it's worth documenting the rationale inline since it's non-obvious.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/common.ts` around lines 373 - 418, The function sanitizeOpenSeaOverlayMedia currently preserves the original data when all media candidates are blocked (see the hasAnyNonBlockedCandidate check and early return), which is an intentional fallback but non-obvious; add a concise inline comment inside sanitizeOpenSeaOverlayMedia (near the hasAnyNonBlockedCandidate check and the return data) explaining that when every candidate (primary and images) matches isBlockedOpenSeaOverlayUrl we intentionally do not strip all media to avoid removing all content, and reference the related symbols hasAnyNonBlockedCandidate, hasBlockedPrimary, and isBlockedOpenSeaOverlayUrl so readers understand the safety-net rationale.components/waves/marketplace/useMarketplacePreviewState.ts (2)
26-33:toNonEmptyStringduplicatesasNonEmptyStringfromlib/text/nonEmptyString.ts.The logic is identical to the shared utility already imported (transitively) in
common.ts. Consider importingasNonEmptyStringdirectly instead of maintaining a local copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/useMarketplacePreviewState.ts` around lines 26 - 33, toNonEmptyString duplicates existing helper asNonEmptyString; replace the local function by importing and using asNonEmptyString (from the shared non-empty string utility) in useMarketplacePreviewState.ts, remove the local toNonEmptyString definition, update any references to call asNonEmptyString instead, and add the necessary import at top so the module uses the shared utility (ensure named import matches the exported symbol asNonEmptyString).
82-90: Successful query with null media is silently mapped to an error state.If the
queryFnresolves successfully but the finalMarketplacePreviewData.mediaisnull(e.g., NFT link has title but no media, and OG fallback also has no image), execution falls through to lines 111–117 wheremarketplacePreviewQuery.errorisnull.toError(null, …)then fabricates a syntheticError("Failed to load marketplace preview").This might be intentional (media is required for a useful preview), but the synthetic error message is misleading — the fetch didn't fail; the data simply lacked media. Consider either returning a distinct state (e.g.,
"no-media") or using a more accurate error message.Also applies to: 92-97, 99-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/useMarketplacePreviewState.ts` around lines 82 - 90, The code is treating a successful fetch with MarketplacePreviewData.media === null as a generic error by passing null into toError and returning type "error"; update the logic in useMarketplacePreviewState (the branch that inspects marketplacePreviewQuery and MarketplacePreviewData) to detect a successful query with null media and return either a distinct state (e.g., type: "no-media" with href and data) or a clearer error (e.g., Error("Preview loaded but no media available")), instead of fabricating Error("Failed to load marketplace preview"); modify the branch that currently calls toError(null, ...) and adjust any callers of marketplace preview state to handle the new "no-media" state (or the new error message) so the UI can render appropriately.components/waves/marketplace/MarketplaceItemPreviewCard.utils.ts (2)
28-31:stopPropagationalso callspreventDefault— name is misleading.The function calls both
event.stopPropagation()andevent.preventDefault(), but the name only suggests propagation stopping. A name likesuppressEventorstopAndPreventwould be clearer for future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceItemPreviewCard.utils.ts` around lines 28 - 31, The exported function stopPropagation currently calls both event.stopPropagation() and event.preventDefault(), so rename it to a clearer identifier (e.g., stopAndPrevent or suppressEvent) and update its export and all imports/usages to the new name; ensure the function body remains the same and adjust any tests/types that reference stopPropagation, or alternatively add a short wrapper named stopPropagation that calls the new function to preserve backward compatibility while updating callers.
33-52: Adding adefaultexhaustiveness guard is optional but recommended for robustness.
noImplicitReturnsis already enabled in tsconfig.json, which means the compiler will enforce that all code paths return a value. However, adding an explicitdefaultbranch with an exhaustiveness check (const _exhaustive: never = kind) provides additional safety that's independent of compiler settings and improves maintainability for future developers.All current
MarketplaceUrlKindvariants are covered:
- "manifold.listing", "superrare.artwork", "foundation.mint"
- "opensea.item", "opensea.asset"
- "transient.nft", "transient.mint"
- null
Consider adding the default branch as an extra layer of protection:
Suggested addition
case null: return null; + default: { + const _exhaustive: never = kind; + return null; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/MarketplaceItemPreviewCard.utils.ts` around lines 33 - 52, Add an explicit default branch to the switch in getMarketplaceBrand to guard against unhandled MarketplaceUrlKind values: in the default case assign the `kind` to a `never`-typed variable (e.g., `const _exhaustive: never = kind`) to get a compile-time exhaustiveness check and then return `null` (or throw a clear error) so all control paths return a value; this change should be made inside the getMarketplaceBrand function to ensure future unknown kinds are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/components/user/utils/icons/FoundationIcon.test.tsx`:
- Line 9: The test expectation in FoundationIcon.test.tsx is asserting the wrong
src value; update the assertion that checks expect(img).toHaveAttribute('src',
'/foundation-icon.jpg') to match the actual component output from
FoundationIcon.tsx (use '/foundation.png') so the test reflects the component's
rendered <Image src="/foundation.png" ... />; locate the check in the test file
and replace the expected string accordingly.
In @.agents/react-doctor/SKILL.md:
- Line 14: Replace the unpinned npx invocation that uses react-doctor@latest
with a pinned explicit version (e.g., react-doctor@<stable-version>) in the
SKILL.md entry that currently contains the string "npx -y react-doctor@latest .
--verbose --diff"; update that command so the package is fixed to a specific
release and add a short note to bump the pinned version periodically as part of
dependency maintenance.
In `@components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.tsx`:
- Around line 6-10: The render path for createNftMarketplacesHandler currently
renders <MarketplacePreview href={href} /> but the test expects compact mode;
update the render function in createNftMarketplacesHandler (the object with
match: isNftMarketplaceLink and render: ...) to pass compact={true} to
MarketplacePreview so the mock receives { href, compact: true } when rendered.
In `@components/waves/marketplace/common.ts`:
- Line 272: In primeMarketplacePreviewCacheFromNftLinks the guard `if
(!seededPreview.media) return;` wrongly exits the whole function when a single
NFT link lacks media; change this to `continue` so only the current iteration is
skipped and the loop proceeds to process remaining NFT links (locate the loop
iterating over NFT links and replace the early `return` in the
seededPreview.media check with `continue`).
In `@components/waves/marketplace/MarketplaceCompactCta.tsx`:
- Around line 62-67: The test id "manifold-item-price" is misleading; update the
span in MarketplaceCompactCta (the element rendering {normalizedPrice}) to use
data-testid="marketplace-item-price", do the same change for the equivalent span
in MarketplaceFullFooter, and update the seven assertions in
MarketplaceItemPreviewCard.test.tsx to reference "marketplace-item-price"
instead of "manifold-item-price" so tests and components consistently use the
generic marketplace test id.
---
Duplicate comments:
In
`@__tests__/components/drops/view/part/dropPartMarkdown/handlers/nftMarketplaces.test.tsx`:
- Around line 140-156: The test fails because createNftMarketplacesHandler's
render (in nftMarketplaces.tsx) returns a MarketplacePreview without the compact
prop; update the render implementation in createNftMarketplacesHandler / render
to pass compact={true} when constructing the MarketplacePreview (i.e., ensure
the component invocation created by render includes compact: true along with
href) so the test expecting { href, compact: true } and the data-href attribute
match.
In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx`:
- Line 112: The code assigns authorHandle using author.handle without guarding
for undefined; update the WaveLeaderboardGridItem assignment so it uses optional
chaining on the author object (e.g., compute authorHandle from author?.handle or
from drop?.author?.handle) and fallback to null if missing; locate the
authorHandle declaration in WaveLeaderboardGridItem and change it to use
optional chaining on author/drop.author to avoid the TypeError when author is
undefined.
In `@openapi.yaml`:
- Around line 2051-2057: The 200 response for the /nft-link endpoint defines an
"items" schema without declaring the container type; update the response schema
(the /nft-link 200 response block) to include "type: array" alongside the
existing "items" so the schema becomes a valid OpenAPI array response
referencing "#/components/schemas/ApiNftLinkResponse".
---
Nitpick comments:
In `@__tests__/components/waves/marketplace/common.test.ts`:
- Around line 8-9: Export the query-key constant and helper from the production
module and import them into the test to eliminate the duplicated implementation:
add exports for MARKETING_PREVIEW_QUERY_KEY (named in production as
MARKETPLACE_PREVIEW_QUERY_KEY) and the helper getMarketplacePreviewQueryKey in
the production common.ts (the same symbols used to build
[MARKETPLACE_PREVIEW_QUERY_KEY, { href, mode }]) and update
__tests__/components/waves/marketplace/common.test.ts to import and use those
exports instead of its local const; ensure the exported helper returns the exact
tuple shape used in production so tests remain in sync.
In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx`:
- Around line 129-139: The IIFE for cardClassName contains dead conditional
branches (isCompactMode, isMarketplaceImageOnlyCard, and the default all return
the same Tailwind string); replace the IIFE with a single constant assignment
for cardClassName using that common class string and remove the unused
conditional checks/branches to simplify the component (refer to cardClassName,
isCompactMode, and isMarketplaceImageOnlyCard to locate and remove the redundant
logic).
- Around line 309-336: The rendering duplicates the same LinkPreviewProvider +
WaveDropPartContentMarkdown subtree in both branches of the
shouldPadContentOnlyText conditional; refactor by computing the shared subtree
(the LinkPreviewProvider wrapping WaveDropPartContentMarkdown with props
mentionedUsers={drop.mentioned_users}, mentionedWaves={drop.mentioned_waves},
referencedNfts={drop.referenced_nfts}, part={activePart}, wave={drop.wave},
drop={drop}, onQuoteClick={() => {}}) once and then conditionally wrap that
subtree with a div className="tw-p-2" when shouldPadContentOnlyText is true,
leaving the existing conditional rendering of activePart unchanged.
In `@components/waves/marketplace/common.ts`:
- Around line 373-418: The function sanitizeOpenSeaOverlayMedia currently
preserves the original data when all media candidates are blocked (see the
hasAnyNonBlockedCandidate check and early return), which is an intentional
fallback but non-obvious; add a concise inline comment inside
sanitizeOpenSeaOverlayMedia (near the hasAnyNonBlockedCandidate check and the
return data) explaining that when every candidate (primary and images) matches
isBlockedOpenSeaOverlayUrl we intentionally do not strip all media to avoid
removing all content, and reference the related symbols
hasAnyNonBlockedCandidate, hasBlockedPrimary, and isBlockedOpenSeaOverlayUrl so
readers understand the safety-net rationale.
In `@components/waves/marketplace/MarketplaceCopyListingButton.tsx`:
- Around line 16-29: The copyToClipboard handler currently uses setTimeout
without cleanup causing stale updates and multiple timers; change this by
creating a timerRef via useRef<number | null>(null), replace setTimeout calls
with assigning window.setTimeout(...) to timerRef.current, clear any existing
timer before setting a new one, and add a useEffect cleanup that clears
timerRef.current on unmount; update references to setCopied and copied as before
and keep stopPropagation(event)/navigator.clipboard.writeText(href) logic intact
in copyToClipboard.
In `@components/waves/marketplace/MarketplaceItemPreviewCard.utils.ts`:
- Around line 28-31: The exported function stopPropagation currently calls both
event.stopPropagation() and event.preventDefault(), so rename it to a clearer
identifier (e.g., stopAndPrevent or suppressEvent) and update its export and all
imports/usages to the new name; ensure the function body remains the same and
adjust any tests/types that reference stopPropagation, or alternatively add a
short wrapper named stopPropagation that calls the new function to preserve
backward compatibility while updating callers.
- Around line 33-52: Add an explicit default branch to the switch in
getMarketplaceBrand to guard against unhandled MarketplaceUrlKind values: in the
default case assign the `kind` to a `never`-typed variable (e.g., `const
_exhaustive: never = kind`) to get a compile-time exhaustiveness check and then
return `null` (or throw a clear error) so all control paths return a value; this
change should be made inside the getMarketplaceBrand function to ensure future
unknown kinds are caught.
In `@components/waves/marketplace/MarketplaceItemPreviewMediaLink.tsx`:
- Around line 28-31: The inner media wrapper in MarketplaceItemPreviewMediaLink
uses a Manifold-specific test ID ("manifold-item-media") but this component
serves multiple marketplaces; change the data-testid on the div with class
`${MARKETPLACE_MEDIA_FRAME_CLASS} tw-relative` inside the
MarketplaceItemPreviewMediaLink component to "marketplace-item-media" so it
matches the outer Link's "marketplace-item-media-link" naming and is
marketplace-agnostic.
In `@components/waves/marketplace/MarketplacePreviewPlaceholder.tsx`:
- Around line 15-18: The explicit variant prop forwarded to
LinkPreviewCardLayout is redundant because LinkPreviewCardLayout already calls
useLinkPreviewVariant() and falls back to context; remove the prop forwarding
(drop variant={variant} from the LinkPreviewCardLayout JSX in
MarketplacePreviewPlaceholder) and keep or remove the local
useLinkPreviewVariant() call depending on usage—if MarketplacePreviewPlaceholder
uses variant for className or other local computation, keep the hook and only
remove the prop; if not, delete the hook call entirely. Ensure references to
useLinkPreviewVariant and LinkPreviewCardLayout (and the component
MarketplacePreviewPlaceholder) are updated accordingly.
In `@components/waves/marketplace/MarketplaceUnavailableCard.tsx`:
- Around line 19-24: The href resolution logic in MarketplaceUnavailableCard
(the block computing relative, isRelativeHref, resolvedHref, target, rel)
duplicates resolvePreviewHref from MarketplaceItemPreviewCard.utils.ts; replace
the manual logic by importing and calling resolvePreviewHref to get the resolved
href and target/rel behavior (or adapt its return to derive target/rel) so both
components share the same implementation and eliminate duplication.
In `@components/waves/marketplace/useMarketplacePreviewState.ts`:
- Around line 26-33: toNonEmptyString duplicates existing helper
asNonEmptyString; replace the local function by importing and using
asNonEmptyString (from the shared non-empty string utility) in
useMarketplacePreviewState.ts, remove the local toNonEmptyString definition,
update any references to call asNonEmptyString instead, and add the necessary
import at top so the module uses the shared utility (ensure named import matches
the exported symbol asNonEmptyString).
- Around line 82-90: The code is treating a successful fetch with
MarketplacePreviewData.media === null as a generic error by passing null into
toError and returning type "error"; update the logic in
useMarketplacePreviewState (the branch that inspects marketplacePreviewQuery and
MarketplacePreviewData) to detect a successful query with null media and return
either a distinct state (e.g., type: "no-media" with href and data) or a clearer
error (e.g., Error("Preview loaded but no media available")), instead of
fabricating Error("Failed to load marketplace preview"); modify the branch that
currently calls toError(null, ...) and adjust any callers of marketplace preview
state to handle the new "no-media" state (or the new error message) so the UI
can render appropriately.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx (1)
227-254: Both branches ofshouldPadContentOnlyTextrender duplicate JSX — extract the wrapper instead.The only structural difference between the two branches is the extra
<div className="tw-p-2">wrapper; theLinkPreviewProvider(functionally equivalent in both positions as a pure context provider) and allWaveDropPartContentMarkdownprops are identical. This can be collapsed into a single render path.♻️ Proposed refactor
- {activePart && - (shouldPadContentOnlyText ? ( - <div className="tw-p-2"> - <LinkPreviewProvider variant="home"> - <WaveDropPartContentMarkdown - mentionedUsers={drop.mentioned_users} - mentionedWaves={drop.mentioned_waves} - referencedNfts={drop.referenced_nfts} - part={activePart} - wave={drop.wave} - drop={drop} - onQuoteClick={() => {}} - /> - </LinkPreviewProvider> - </div> - ) : ( - <LinkPreviewProvider variant="home"> - <WaveDropPartContentMarkdown - mentionedUsers={drop.mentioned_users} - mentionedWaves={drop.mentioned_waves} - referencedNfts={drop.referenced_nfts} - part={activePart} - wave={drop.wave} - drop={drop} - onQuoteClick={() => {}} - /> - </LinkPreviewProvider> - ))} + {activePart && ( + <LinkPreviewProvider variant="home"> + <div className={shouldPadContentOnlyText ? "tw-p-2" : undefined}> + <WaveDropPartContentMarkdown + mentionedUsers={drop.mentioned_users} + mentionedWaves={drop.mentioned_waves} + referencedNfts={drop.referenced_nfts} + part={activePart} + wave={drop.wave} + drop={drop} + onQuoteClick={() => {}} + /> + </div> + </LinkPreviewProvider> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx` around lines 227 - 254, The JSX duplicates for both branches of shouldPadContentOnlyText; consolidate by rendering LinkPreviewProvider with WaveDropPartContentMarkdown once (using activePart, drop, WaveDropPartContentMarkdown props mentionedUsers/mentionedWaves/referencedNfts/part/wave/drop/onQuoteClick) and conditionally wrap that single element with <div className="tw-p-2"> when shouldPadContentOnlyText is true; implement this refactor in WaveLeaderboardGridItem so the provider and WaveDropPartContentMarkdown are not duplicated.components/waves/marketplace/common.ts (1)
36-39:PickedMedianot exported despite being part of the public API surface.
PickedMediaappears in two exported types (MarketplacePreviewData.mediaandMarketplacePreviewState['resolvedMedia']), so callers who want to type-annotate a variable holding the media value must resort to workarounds likeNonNullable<MarketplacePreviewData['media']>instead of a direct import.♻️ Proposed change
-type PickedMedia = { +export type PickedMedia = { readonly url: string; readonly mimeType: string; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/marketplace/common.ts` around lines 36 - 39, The type PickedMedia is currently internal but is referenced by exported types MarketplacePreviewData.media and MarketplacePreviewState['resolvedMedia']; make it part of the public API by exporting it (e.g., change "type PickedMedia" to "export type PickedMedia") so callers can import it directly and avoid workarounds like NonNullable<MarketplacePreviewData['media']>.components/waves/CreateCurationDropContent.tsx (1)
388-403: RedundantisInitialMountRef.current = falseon line 395.Line 392 sets the ref to
falseand returns early (app + initial mount). Line 395 runs only when the early-return branch was skipped; for non-app environments it correctly clears the flag on first run, but on every subsequent render it is a harmless no-op. Hoisting the assignment before theifor restructuring the guard makes the intent clearer:♻️ Suggested cleanup
- // Skip auto-focus on initial mount in app to prevent keyboard from opening - if (isApp && isInitialMountRef.current) { - isInitialMountRef.current = false; - return; - } - isInitialMountRef.current = false; + const wasInitialMount = isInitialMountRef.current; + isInitialMountRef.current = false; + // Skip auto-focus on initial mount in app to prevent keyboard from opening + if (isApp && wasInitialMount) { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/CreateCurationDropContent.tsx` around lines 388 - 403, The effect contains a redundant assignment to isInitialMountRef.current; move the assignment before the early-return or remove the second assignment so the ref is set to false exactly once on first run: inside the useEffect that references isInitialMountRef, isApp, activeDrop and inputRef (the effect body around the auto-focus logic), set isInitialMountRef.current = false before the if (isApp && isInitialMountRef.current) guard (or remove the later duplicate) so the early-return branch still prevents focusing but the flag is only cleared once and intent is clearer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/waves/CreateCurationDropContent.tsx`:
- Line 325: In CreateCurationDropContent.tsx replace the unsafe cast on
document.activeElement before calling blur: read document.activeElement into a
local (e.g., const active = document.activeElement) and check that active is
non-null and an HTMLElement (if (active instanceof HTMLElement) active.blur();)
or use safe optional chaining with a proper type guard; this ensures the call in
the component does not throw in environments where activeElement can be null or
a non-HTMLElement.
- Around line 79-107: The useEffect in CreateCurationDropContent captures
onClose which is an unstable inline callback, causing the effect to
cleanup/re-run on every parent render; fix by removing onClose from the
dependency array and instead stabilize access to it: either (A) wrap the
parent's inline onClose (e.g., the () => setIsSupportedUrlsModalOpen(false)
passed into CurationInfoModal) with useCallback so its reference is stable, or
(B) keep onClose unstable but store the latest handler in a ref inside
CreateCurationDropContent (e.g., onCloseRef.current = onClose) and call
onCloseRef.current() from the onKeyDown cleanup and Escape handler, then use
[isApp, isOpen] as the useEffect deps so previousActiveElementRef and modalRef
focus/overflow logic no longer flips.
- Around line 113-136: The transparent backdrop button inside the dialog is
positioned fixed and currently sits above the modal content because the content
<div> is non-positioned so its tw-z-10 is ignored; update the content container
(the div referenced by modalRef that wraps ModalLayout in modalContent) to be
positioned (add tw-relative) so its tw-z-10 takes effect and the modal
(ModalLayout) renders above the backdrop button; keep the backdrop button
behavior and onClose handler unchanged.
In `@components/waves/marketplace/common.ts`:
- Around line 58-66: IMAGE_MIME_BY_EXTENSION is missing video, audio and 3D
model extensions so inferMimeTypeFromUrl (and thus toPickedMedia /
pickMediaFromUrl) falls back to image/* for media_uri that are actually
video/audio/3D; add common video (mp4, webm, mov, ogv, m4v), audio (mp3, ogg,
wav, flac) and 3D model (glb, gltf, usdz) mappings to IMAGE_MIME_BY_EXTENSION
with their proper MIME types (e.g., mp4 -> video/mp4, webm -> video/webm, mov ->
video/quicktime, ogv -> video/ogg, m4v -> video/x-m4v, mp3 -> audio/mpeg, ogg ->
audio/ogg, wav -> audio/wav, flac -> audio/flac, glb -> model/gltf-binary, gltf
-> model/gltf+json, usdz -> model/vnd.usdz) so inferMimeTypeFromUrl returns the
correct media MIME and MediaDisplay.getMediaType() will detect VIDEO/AUDIO/3D
correctly.
---
Duplicate comments:
In `@components/waves/CreateCurationDropContent.tsx`:
- Around line 216-240: The promise created in CreateCurationDropContent.tsx that
dispatches the "showTermsModal" CustomEvent can remain unresolved if
TermsSignatureFlow (the dynamic listener) isn't mounted; add a timed fallback so
the promise always resolves: start a timeout (e.g., 8–10s) before dispatching,
and if it fires resolve(null); when handleSigningComplete is invoked clear the
timeout and resolve as before; ensure any cleanup (clearing timeout) happens in
both success/failure paths so submitting cannot stay stuck waiting for the event
listener.
- Around line 313-315: The optimistic add is firing without awaiting, so
rejections bypass the surrounding try/catch and leave stale UI state; change the
call to await addOptimisticDrop({ drop: optimisticDrop }) inside the existing
try/catch (in CreateCurationDropContent where addOptimisticDrop is invoked) and
handle failures in the catch by rolling back the optimistic state (e.g., call
removeOptimisticDrop or setOptimisticDrop(null)/undefined) and surface an error
(toast or processLogger) so the user isn’t left with stale optimistic UI.
In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx`:
- Around line 49-50: The code assigns const author = drop.author and then reads
author.handle without guarding for undefined; update WaveLeaderboardGridItem to
safely access the handle (e.g., use optional chaining or a null-coalescing
guard) so reading the value won't throw if drop.author is undefined — replace
author.handle ?? null with author?.handle ?? null or add an explicit check for
author before computing authorHandle (referencing drop.author, author, and
authorHandle in WaveLeaderboardGridItem).
In `@components/waves/marketplace/common.ts`:
- Around line 264-284: The loop previously returned early when an NFT link
lacked media; ensure the fix keeps using continue (not return) so only the
current nftLink is skipped — in the block iterating nftLinks, keep the
deduplication via seenHrefs (after normalizeHref), call fromApiDropNftLink to
build seededPreview, and if !seededPreview.media use continue; then fan out
across MARKETPLACE_PREVIEW_MODES and call queryClient.setQueryData with key
[MARKETPLACE_PREVIEW_QUERY_KEY, { href, mode }] and
mergeSeededMarketplacePreviewData to seed the cache without overwriting existing
data.
- Around line 1-7: Consolidate the duplicated string validation by importing the
runtime utility asNonEmptyString from "@/lib/text/nonEmptyString" everywhere
(remove any local duplicates), keep matchesDomainOrSubdomain as a runtime
import, and ensure all type-only imports remain as import type (e.g.,
ApiDropNftLink, WsMediaLinkUpdatedData, LinkPreviewResponse, ApiNftLinkResponse,
QueryClient); update any files that still define or import a second
asNonEmptyString to use the centralized export instead.
---
Nitpick comments:
In `@components/waves/CreateCurationDropContent.tsx`:
- Around line 388-403: The effect contains a redundant assignment to
isInitialMountRef.current; move the assignment before the early-return or remove
the second assignment so the ref is set to false exactly once on first run:
inside the useEffect that references isInitialMountRef, isApp, activeDrop and
inputRef (the effect body around the auto-focus logic), set
isInitialMountRef.current = false before the if (isApp &&
isInitialMountRef.current) guard (or remove the later duplicate) so the
early-return branch still prevents focusing but the flag is only cleared once
and intent is clearer.
In `@components/waves/leaderboard/grid/WaveLeaderboardGridItem.tsx`:
- Around line 227-254: The JSX duplicates for both branches of
shouldPadContentOnlyText; consolidate by rendering LinkPreviewProvider with
WaveDropPartContentMarkdown once (using activePart, drop,
WaveDropPartContentMarkdown props
mentionedUsers/mentionedWaves/referencedNfts/part/wave/drop/onQuoteClick) and
conditionally wrap that single element with <div className="tw-p-2"> when
shouldPadContentOnlyText is true; implement this refactor in
WaveLeaderboardGridItem so the provider and WaveDropPartContentMarkdown are not
duplicated.
In `@components/waves/marketplace/common.ts`:
- Around line 36-39: The type PickedMedia is currently internal but is
referenced by exported types MarketplacePreviewData.media and
MarketplacePreviewState['resolvedMedia']; make it part of the public API by
exporting it (e.g., change "type PickedMedia" to "export type PickedMedia") so
callers can import it directly and avoid workarounds like
NonNullable<MarketplacePreviewData['media']>.
|
ragnep
approved these changes
Feb 20, 2026
ragnep
approved these changes
Feb 20, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Summary by CodeRabbit
New Features
Improvements