Conversation
📝 WalkthroughWalkthroughAdds a resubmission flow, modal wiring for desktop and touch (deferred open after mobile transition), extensive submission form refactor (reducer, actions, media controls, validation), new drop/media preview components and types, deletion flow/hook, and broad test coverage updates for resubmit and preview behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Leaderboard as Leaderboard UI
participant Resubmit as MemesArtResubmitAction
participant MobileMenu as Mobile Dropdown
participant Modal as MemesArtSubmissionModal/Shell
participant Form as Submission Form (hooks)
participant API as Backend/API
User->>Leaderboard: open drop actions
Leaderboard->>Resubmit: render with wave & onSourceDropDeleted
User->>Resubmit: tap/click "Resubmit"
alt mobile menu present
Resubmit->>MobileMenu: setIsActive(false) (close menu)
MobileMenu-->>Resubmit: onAfterLeave (deferred)
end
Resubmit->>Modal: open submission modal (internal or external opener)
User->>Modal: acknowledge / fill form / submit
Modal->>Form: submit resubmission
Form->>API: POST resubmitted artwork
API-->>Form: returns new drop
Form->>Modal: surface delete-original option (if applicable)
User->>Modal: confirm delete original
Modal->>API: DELETE original drop
API-->>Modal: success
Modal->>Leaderboard: call onSourceDropDeleted()
Leaderboard->>Leaderboard: refetch leaderboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 docstrings
🧪 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: 6
🧹 Nitpick comments (19)
components/waves/drop/MemesDropFullscreenOverlay.tsx (1)
18-18:onClosetyped asMouseEventHandlerlimits reuse.Typing
onCloseasMouseEventHandler<HTMLButtonElement>couples the API to a click origin. If you later trigger it from a keyboard handler (ESC) or backdrop click, the signature becomes awkward. Prefer a plain() => void.Proposed change
- readonly onClose: MouseEventHandler<HTMLButtonElement>; + readonly onClose: () => void;And drop the
MouseEventHandlerimport.Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drop/MemesDropFullscreenOverlay.tsx` at line 18, The prop type for onClose in the MemesDropFullscreenOverlay component is too specific; change the onClose signature from MouseEventHandler<HTMLButtonElement> to a simple () => void in the props/interface (reference: onClose and MemesDropFullscreenOverlay) so it can be invoked from clicks, keyboard (ESC) or backdrop events, and remove the now-unused MouseEventHandler import; update any callers to pass a zero-arg function if needed.components/waves/memes/submission/hooks/useResubmissionDelete.ts (1)
59-63: Stale auth check ifrequestAuthruns before setting loading state? Verify ordering.
setIsDeletingOriginal(true)fires beforerequestAuth(), so if auth is cancelled the loading state briefly flashes true then false. Minor UX nit — consider performingrequestAuth()first, then flipping the loading flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useResubmissionDelete.ts` around lines 59 - 63, In useResubmissionDelete change the order around the auth call and loading flag: call requestAuth() and await its result before calling setIsDeletingOriginal(true) so the UI loading state is only set if auth succeeds; specifically, move the requestAuth() invocation earlier (or conditionalize setIsDeletingOriginal) around the existing requestAuth / setIsDeletingOriginal lines to avoid the brief flash when requestAuth is cancelled.components/waves/drop/MemesDropSummarySection.tsx (1)
10-10: Consider makingdescriptionoptional.It is rendered conditionally (
{description && ...}) on line 51, so the string can effectively be empty. Typing it asreadonly description?: string | undefinedmatches usage and removes a requirement for callers to pass an empty string explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drop/MemesDropSummarySection.tsx` at line 10, The prop type for the drop summary currently forces callers to provide description via "readonly description: string;" even though the component checks "{description && ...}" and treats it as optional; change the property on the relevant type/interface to be optional (e.g. "readonly description?: string" or "readonly description?: string | undefined") so callers can omit it and the typing matches runtime usage; update the type declaration where "readonly description" is defined (the interface/type used by MemesDropSummarySection) accordingly.components/waves/drop/memesDropPanelTypes.ts (1)
1-14: Consider naming consistency betweenmime_typeandfileInfo.
MemesDropMediauses snake_case (mime_type) whileMemesDropDownloadDatauses camelCase (fileInfo). Within a file introducing related types, pick one convention (snake_case likely tracks API payloads, camelCase tracks UI-shaped data — splitting them by origin is fine, but worth a quick look to confirm the intent here).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drop/memesDropPanelTypes.ts` around lines 1 - 14, MemesDropMedia currently defines mime_type (snake_case) while MemesDropDownloadData and MemesDropFileInfo use camelCase (fileInfo, name, extension); make the naming consistent by choosing one convention (e.g., convert mime_type to mimeType or convert fileInfo to file_info), update the interfaces (MemesDropMedia, MemesDropFileInfo, MemesDropDownloadData) accordingly, and then update all usages of those symbols (references to mime_type/mimeType and fileInfo/file_info) throughout the codebase to match the chosen convention so types align with their origin (API payload vs UI model).__tests__/components/waves/memes/submission/utils/resubmissionMediaType.test.ts (1)
3-31: Consider broadening coverage to other branches ofgetResubmissionMediaTypeInfo.The suite only covers GLTF/GLB paths. The implementation also handles
video/*,text/html, unknown types ({ label: null, isInteractive: false }), extension-with-query-string stripping, and case-insensitive normalization — all untested here.Suggested additional cases
it("classifies video mime types", () => { expect(getResubmissionMediaTypeInfo({ mimeType: "video/mp4" })).toEqual({ label: "Video", isInteractive: false, }); }); it("classifies text/html as interactive HTML", () => { expect(getResubmissionMediaTypeInfo({ mimeType: "text/html" })).toEqual({ label: "HTML", isInteractive: true, }); }); it("returns null label for unknown types", () => { expect(getResubmissionMediaTypeInfo({ mimeType: "image/png" })).toEqual({ label: null, isInteractive: false, }); }); it("ignores query strings and fragments when matching extensions", () => { expect( getResubmissionMediaTypeInfo({ url: "https://x/y.GLB?v=1#frag" }) ).toEqual({ label: "GLB", isInteractive: true }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/memes/submission/utils/resubmissionMediaType.test.ts` around lines 3 - 31, Add unit tests covering the other branches of getResubmissionMediaTypeInfo: add cases that pass mimeType "video/mp4" expecting { label: "Video", isInteractive: false }, mimeType "text/html" expecting { label: "HTML", isInteractive: true }, an unknown mimeType like "image/png" expecting { label: null, isInteractive: false }, and a URL with mixed case and query/fragment like "https://x/y.GLB?v=1#frag" expecting { label: "GLB", isInteractive: true }; place these new it(...) blocks alongside the existing GLTF/GLB tests in the same test file to exercise extension stripping and case-insensitive normalization in getResubmissionMediaTypeInfo.components/waves/memes/submission/MemesArtSubmissionShell.tsx (1)
27-39: Optional: dropmotion.*wrappers if no animation is applied.
motion.h3andmotion.buttonare used without anyinitial/animate/exit/transitionprops, so they add framer-motion overhead with no behavioral benefit. Plainh3/buttonwould suffice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/MemesArtSubmissionShell.tsx` around lines 27 - 39, The motion wrappers around the header and close button in MemesArtSubmissionShell (motion.h3 and motion.button) provide no animation props and should be replaced with plain h3 and button elements (preserve title, className, aria-label, onClose handler and inner FontAwesomeIcon) to avoid framer-motion overhead; after replacing, remove any unused framer-motion imports (e.g., motion) from the file to clean up unused dependencies.__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx (1)
101-112: BrittleuseStatespy.
jest.spyOn(React, "useState").mockImplementationOnce(...)depends onuseStatebeing the very first hook invoked on render. Any reordering or added hook upstream inMemesSingleWaveDropInfoPanelwill silently break this test (it will target the wrong state slot). Consider testing fullscreen behavior through user interaction (clicking an "Enter fullscreen" affordance) instead of mocking React internals. Noting as pre-existing pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx` around lines 101 - 112, The test is brittle because it spies on React.useState; remove the useState spy in the "closes fullscreen when button clicked" test and instead exercise the component through user interactions: render MemesSingleWaveDropInfoPanel, simulate the user clicking the "Enter fullscreen view" control (or whatever affordance opens fullscreen), then click the "Exit fullscreen view" button via screen.getByRole and assert the visible outcome (e.g., fullscreen element is removed, a particular aria-hidden/aria-expanded state changes, or the Exit button is no longer in the document) rather than asserting a mocked setState was called.components/waves/memes/file-upload/components/FilePreview.tsx (1)
45-56: Consider URL-extension fallback for GLB detection on resubmission.
isGLBstill relies onfile?.namefor extension-based detection. When resubmitting existing GLB media (nofile, onlyurl+mimeType), detection works only ifmimeTypeis correctlymodel/gltf-binary/model/gltf+json. If the existing media URL ends in.glb/.gltfbut has a generic/incorrectmimeType, the preview will miss the GLB branch. Consider falling back to extracting the extension fromurlsimilar to the helper inresubmissionMediaType.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/file-upload/components/FilePreview.tsx` around lines 45 - 56, The isGLB detection currently checks effectiveMimeType and file?.name extensions but misses the case where there's no file and the resubmitted media URL ends with .glb/.gltf; update the isGLB logic in FilePreview.tsx (symbols: effectiveMimeType, isGLB, url, file) to also inspect the url's pathname/extension when file is undefined or file?.name is absent—e.g., extract the extension from url (similar to the helper in resubmissionMediaType.ts) and treat .glb/.gltf as GLB so resubmitted GLB URLs with incorrect/blank mimeType still enter the GLB branch.components/waves/drop/MemesDropDetailsSection.tsx (1)
39-41: Nit:artworkMedia !== nullallowsundefinedthrough.
artworkMediais typed asMemesDropMedia | null | undefined, so!== nullistruewhenartworkMediaisundefined. It's safe today becausefileInfowill benullwheneverartworkMediais falsy (per the parentMemesSingleWaveDropInfoPanel), but a truthy check is more defensive:🛡️ Proposed tweak
- const hasArtworkDownload = artworkMedia !== null && fileInfo !== null; + const hasArtworkDownload = Boolean(artworkMedia) && fileInfo !== null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drop/MemesDropDetailsSection.tsx` around lines 39 - 41, Change the null-only check for artworkMedia to a defensive truthy/nonnull check so undefined doesn’t slip through: update the hasArtworkDownload expression (which currently uses artworkMedia !== null && fileInfo !== null) to ensure artworkMedia is present (e.g., artworkMedia != null or a truthy check like Boolean(artworkMedia)) before checking fileInfo; keep hasDownloads logic using previewImageData and promoVideoData unchanged besides using consistent null/undefined-safe checks if desired.__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx (1)
175-201: LGTM — deferred-modal behavior is well captured.The test correctly asserts that clicking the resubmit action calls
setIsActive(false)without opening the modal, and that the modal only appears after the mockedafter-leavetrigger — matching the productionafterLeavewiring onTransitionChild.Minor observation: the
CommonDropdownItemsMobileWrappermock renders its children unconditionally, which means this test doesn't enforce that the resubmit action lives behind the transition. If you want to harden the invariant, guard the children with{p.isOpen ? p.children : null}in the mock.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx` around lines 175 - 201, The CommonDropdownItemsMobileWrapper test mock currently renders children unconditionally which bypasses the Transition gating; update the mock used in this test to only render children when the wrapper's isOpen prop is true (i.e., change the mock component CommonDropdownItemsMobileWrapper to return p.isOpen ? p.children : null) so the resubmit action is actually behind the open/close transition and the test enforces the production behavior in MemesLeaderboardDrop.components/waves/memes/submission/hooks/useArtworkSubmissionMutation.ts (1)
25-43: LGTM onexistingMediaextension.The validation (
hasFile || hasExisting || hasExternal), the explicitmimeTypecheck forexistingMedia, and the payload construction branch (url/mime_type) are consistent withApiDropMedia. The normalizedexistingUrl = data.existingMedia?.url.trim() ?? ""keeps the control flow safe whenexistingMediais absent.One small nit:
externalMedia.mimeTypewas narrowed toInteractiveMediaMimeType; widening it tostringhere removes a type-level guard at the call sites. If the only caller that passes a non-interactive value is theexistingMedia/resubmission path, consider keepingexternalMedia.mimeTypetyped asInteractiveMediaMimeTypeand usingexistingMedia.mimeType: stringfor the resubmission mime-type passthrough (which is what you already do).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionMutation.ts` around lines 25 - 43, Update the ArtworkSubmissionData type so externalMedia.mimeType stays narrowed to InteractiveMediaMimeType while existingMedia.mimeType is a plain string: change externalMedia?: { url: string; mimeType: InteractiveMediaMimeType } | undefined and existingMedia?: { url: string; mimeType: string } | undefined; leave all other fields and the normalized existingUrl logic untouched so callers that require interactive-only mime types still get type-level guarantees while resubmission passthroughs accept arbitrary existingMedia mime strings.components/waves/memes/submission/hooks/useArtworkSubmissionFormActions.ts (2)
48-51: Nit: double-applying profile defaults onhandleContinueFromArtwork.
applyProfileDefaultsruns again here in addition tohandleContinueFromTerms.reduceProfileDefaultsis idempotent (only fills empty fields), so this is safe, but if it isn't intentional (e.g., to cover cases where user skipped terms or re-entered the flow), the second call could be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionFormActions.ts` around lines 48 - 51, The handler handleContinueFromArtwork currently calls applyProfileDefaults again even though applyProfileDefaults is already invoked in handleContinueFromTerms and reduceProfileDefaults is idempotent; remove the duplicate applyProfileDefaults call from handleContinueFromArtwork so it only dispatches SET_STEP to SubmissionStep.ADDITIONAL_INFO (leave applyProfileDefaults in handleContinueFromTerms as the single source of default application), referencing the functions handleContinueFromArtwork, handleContinueFromTerms, applyProfileDefaults and reduceProfileDefaults when making the change.
123-134: Redundant re-assignment oftitleanddescription.Both fields are already in
traitsvia the spread; the explicittitle: traits.title/description: traits.descriptionlines are no-ops. Safe to drop for clarity.♻️ Proposed diff
const getSubmissionData = useCallback(() => { const { artworkUrl, operationalData, traits } = state; return { imageUrl: artworkUrl, - traits: { - ...traits, - title: traits.title, - description: traits.description, - }, + traits: { ...traits }, operationalData, }; }, [state]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionFormActions.ts` around lines 123 - 134, The getSubmissionData function redundantly reassigns traits.title and traits.description after spreading traits; remove the no-op explicit entries and return traits directly from state.traits. Locate the getSubmissionData useCallback (references: getSubmissionData, state, artworkUrl, traits, operationalData) and update its return to include imageUrl: artworkUrl, traits: { ...traits }, operationalData (or simply traits), eliminating title: traits.title and description: traits.description.components/waves/memes/submission/MemesArtResubmitAction.tsx (1)
115-131:aria-disabledonly — verify no interactive side effects for disabled users.The buttons set
aria-disabledbut not the nativedisabledattribute (presumably so the title/tooltip remains accessible).openModalcorrectly guards withif (disabled) return;, but note:
- Keyboard users activating via Enter/Space will hit the
onClickearly-return path silently (no error feedback beyond the static inline reason).- The
disabledReasonparagraphs inmenu/buttonvariants aren't programmatically associated with the control (noaria-describedby), so assistive tech won't announce the reason.Non-blocking, but consider wiring an
aria-describedby={tooltipId}/inline-reason id whendisabledis true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/MemesArtResubmitAction.tsx` around lines 115 - 131, The button currently uses aria-disabled but doesn't associate the visible disabled reason with the control; create a stable id for the disabled message (e.g., tooltipId) and render that id on the disabledReason paragraph in the menu/button variants, then set aria-describedby={tooltipId} on the interactive element when disabled is true; keep the existing openModal guard (if (disabled) return) and preserve onOpenModal logic so keyboard activation still short-circuits safely while assistive tech can announce the reason via aria-describedby.components/waves/memes/submission/MemesArtSubmissionStepContent.tsx (1)
60-84: Consider memoizing the media-type derivation.
mediaTypeInfois computed in an IIFE on every render. It's cheap today, but memoizing keeps the contract stable for downstream components that receivepreviewRequiredMediaType/requiresPreviewImage/requiresPromoVideoOptionas props and would otherwise see a fresh object reference on every render.♻️ Proposed refactor
- const mediaTypeInfo = (() => { - if (form.mediaSource === "upload" && form.selectedFile) { - return getResubmissionMediaTypeInfo({ - mimeType: form.selectedFile.type, - fileName: form.selectedFile.name, - }); - } - if (form.mediaSource === "upload" && form.existingMedia) { - return getResubmissionMediaTypeInfo({ - mimeType: form.existingMedia.mimeType, - url: form.existingMedia.url, - }); - } - if (form.mediaSource === "url" && form.isExternalMediaValid) { - return getResubmissionMediaTypeInfo({ - mimeType: form.externalMediaMimeType, - url: form.externalMediaUrl, - }); - } - return { label: null, isInteractive: false }; - })(); + const mediaTypeInfo = useMemo(() => { + if (form.mediaSource === "upload" && form.selectedFile) { + return getResubmissionMediaTypeInfo({ + mimeType: form.selectedFile.type, + fileName: form.selectedFile.name, + }); + } + if (form.mediaSource === "upload" && form.existingMedia) { + return getResubmissionMediaTypeInfo({ + mimeType: form.existingMedia.mimeType, + url: form.existingMedia.url, + }); + } + if (form.mediaSource === "url" && form.isExternalMediaValid) { + return getResubmissionMediaTypeInfo({ + mimeType: form.externalMediaMimeType, + url: form.externalMediaUrl, + }); + } + return { label: null, isInteractive: false }; + }, [ + form.mediaSource, + form.selectedFile, + form.existingMedia, + form.isExternalMediaValid, + form.externalMediaMimeType, + form.externalMediaUrl, + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/MemesArtSubmissionStepContent.tsx` around lines 60 - 84, mediaTypeInfo is recomputed on every render causing a new object identity each time; wrap its computation in React.useMemo to memoize the returned object and stabilize downstream props (use the same logic inside the memo). Use form.mediaSource, form.selectedFile, form.existingMedia, form.isExternalMediaValid, form.externalMediaMimeType and form.externalMediaUrl as the memo dependency list so getResubmissionMediaTypeInfo is only recalculated when inputs change, and leave previewRequiredMediaType, requiresPreviewImage and requiresPromoVideoOption derived from the memoized mediaTypeInfo as before.components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts (1)
91-110: Clarify:setArtworkUploaded(true)is a no-op.The name suggests a setter, but
uploaded=trueearly-returns whilefalseresets upload/external state. Consider renaming (e.g.clearArtwork/resetUploadedArtwork) or at least documenting thattrueis intentionally a no-op, sinceSET_UPLOAD_MEDIAis the only path that setsartworkUploaded: true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts` around lines 91 - 110, The function setArtworkUploaded in useArtworkSubmissionMediaControls is misleading because setArtworkUploaded(true) is a no-op while false clears state; rename it to resetUploadedArtwork or clearArtwork (and update all callers) to reflect that it clears upload/external state, or alternatively add an inline comment above setArtworkUploaded documenting that true intentionally no-ops and only false triggers reset; when renaming, update the function reference in the dependency array and replace usages that call setArtworkUploaded(true)/false, and ensure the implementation still calls updateExternalMediaState("", state.externalMedia.provider) for mediaSource === "url" and dispatch({ type: "RESET_UPLOAD_MEDIA" }) otherwise.components/waves/memes/submission/utils/submissionDraft.ts (2)
88-117: Clarify trait override for title/description.If
title/descriptionare present intraitDefinitions(they are, given they're included ingetInitialTraitsValues), the loop at Line 88–108 assigns them frommetadata.get("title"|"description")first, then Line 110–114 re‑overrides them. This is not a bug (same metadata source), but the fallback todrop.title/drop.parts.at(0)?.contentonly fires when metadata lacks these keys. Consider skippingtitle/descriptionin the loop or adding a brief comment noting the intentional re-assignment for thedrop.*fallback path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/utils/submissionDraft.ts` around lines 88 - 117, The code currently sets title/description inside the fieldDefinitions.forEach loop from metadata.get(...) and then unconditionally reassigns traits.title and traits.description from metadata.get(...) ?? drop.title ?? ... which is redundant and obscures the intent; update the fieldDefinitions processing in submissionDraft.ts to skip the "title" and "description" fields (check definition.field === "title" || "description") so they are only resolved by the explicit fallback assignment, or alternatively add a short inline comment above the final assignments (traits.title / traits.description) explaining that title/description are intentionally handled after the loop to allow drop.title and drop.parts fallback when metadata is absent; reference the loop over fieldDefinitions, metadata.get, and the final traits.title/traits.description assignments when making the change.
64-72: Nit:booleanValuetreats many unrelated strings asfalse.Anything not matching
true/1/yesbecomesfalse, including stored values likeonor locale-specific encodings. If the producer only ever writes"true"/"false", this is fine — otherwise consider rejecting ambiguous inputs back to the trait default (mirroringnumberValue's fallback pattern). Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/utils/submissionDraft.ts` around lines 64 - 72, The current booleanValue function coerces any non-true-like string to false; change booleanValue to accept a fallback parameter (like numberValue) and return that fallback for ambiguous inputs: normalize the input string, explicitly check for true-like tokens ("true","1","yes","on") to return true, check for false-like tokens ("false","0","no","off") to return false, and otherwise return the provided fallback; update all call sites to pass the appropriate trait default or leave a safe default when calling booleanValue.components/waves/memes/submission/MemesArtSubmissionContainer.tsx (1)
241-302: Nit: shelldescriptiononly applies to the non-preview step path.
shellDescriptionis non-null only whenisResubmission && hasAcknowledgedResubmission && !replacementDrop, i.e. while filling the form. During theResubmitAcknowledgementandResubmitDeleteConfirmationsubsteps the shell has no description, which is intentional per the flow — just flagging so it's considered if those screens would benefit from their own header subtext. No change required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/MemesArtSubmissionContainer.tsx` around lines 241 - 302, shellDescription is only set for the form path (when isResubmission && hasAcknowledgedResubmission && !replacementDrop), so ResubmitAcknowledgement and ResubmitDeleteConfirmation render without shell description; if you want consistent header subtext either compute shellDescription to cover those substeps (e.g., branch on isResubmission + specific substep flags) or have ResubmitAcknowledgement and ResubmitDeleteConfirmation accept a description prop and pass it through when rendering inside MemesArtSubmissionShell; update the logic around shellDescription (and the render of ResubmitAcknowledgement / ResubmitDeleteConfirmation) in MemesArtSubmissionContainer to supply the desired text.
🤖 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/memes/drops/MemesLeaderboardDrop.tsx`:
- Around line 272-279: The mobile path is missing a callback so deleting the
original after a resubmit doesn’t notify the leaderboard; add an optional
onSourceDropDeleted prop to the MemesLeaderboardDrop component signature and its
props interface, thread that prop through where the resubmit modal is opened
(use the existing isResubmitModalOpen state and setIsResubmitModalOpen setter),
and pass onSourceDropDeleted into the MemesArtSubmissionModal invocation so the
leaderboard can react (close/remove card or invalidate queries) when the
original drop is deleted.
In `@components/waves/drop/MemesDropFullscreenOverlay.tsx`:
- Around line 29-78: MemesDropFullscreenOverlay currently only closes via the
close button (props/state: isOpen, onClose, artworkMedia) so add an Escape key
handler and backdrop-click close and implement focus trap/restore: attach a
useEffect in MemesDropFullscreenOverlay that adds a keydown listener for
"Escape" to call onClose when isOpen is true, ensure the backdrop (the
motion.div or an outer backdrop element) handles clicks and only closes when
clicking outside the inner content area, and add focus management (trap focus
inside the overlay while open and restore focus to the previously focused
element on close) either by integrating a Dialog primitive (e.g., Headless UI
Dialog) or using a focus-trap utility inside the component. Ensure you clean up
listeners on unmount/close and that onClose is invoked consistently.
In `@components/waves/memes/submission/hooks/artworkSubmissionFormState.ts`:
- Around line 625-637: The reducer reduceResetUploadMedia currently nulls out
existingMedia (used to revert to the drop's original media) on
RESET_UPLOAD_MEDIA; change it to preserve existingMedia unless the user
explicitly opted out, i.e., remove the existingMedia: null assignment and only
clear selectedFile, uploadArtworkUrl (and set artworkUrl/artworkUploaded based
on shouldFallbackToExternal) so resubmission can revert to original media; if
nulling was intentional, add a test/comment in reduceResetUploadMedia explaining
the explicit opt-out behavior.
In
`@components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts`:
- Around line 77-89: The handleFileSelect callback currently ignores FileReader
errors/aborts; add reader.onerror and reader.onabort handlers alongside
reader.onloadend to dispatch an error state (or show a toast) so failures are
surfaced to the user. Specifically, in useArtworkSubmissionMediaControls'
handleFileSelect, keep the existing reader.onloadend dispatch for successful
reads but also dispatch a new action (e.g., type: "SET_UPLOAD_ERROR" or reuse
existing form invalid mechanism) with an error message when reader.onerror or
reader.onabort fires, and ensure any cleanup/reset of upload state (clearing
file/artworkUrl) happens in those handlers so the UI doesn't retain old state.
In `@components/waves/memes/submission/hooks/useResubmissionDelete.ts`:
- Around line 94-99: The finally block is returning which can swallow errors;
remove the return-from-finally pattern in the finally and instead reset
setIsDeletingOriginal(false) in the explicit success and error paths of the
async flow inside useResubmissionDelete; keep the early onClose() behavior (when
didClose is true) and avoid calling setIsDeletingOriginal after onClose (since
onClose may unmount the component), so move the setIsDeletingOriginal(false)
calls to the branches where the operation completes or fails and drop the
finally entirely, referencing the didClose flag, onClose() call, and
setIsDeletingOriginal state updater to locate the code to change.
In `@components/waves/memes/submission/MemesArtSubmissionShell.tsx`:
- Around line 30-39: The close button in the MemesArtSubmissionShell component
(the motion.button with onClick={onClose} and the FontAwesomeIcon) lacks an
explicit type, causing it to default to type="submit" and potentially submit
enclosing forms; update that motion.button element to include type="button" to
prevent accidental form submissions.
---
Nitpick comments:
In `@__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx`:
- Around line 175-201: The CommonDropdownItemsMobileWrapper test mock currently
renders children unconditionally which bypasses the Transition gating; update
the mock used in this test to only render children when the wrapper's isOpen
prop is true (i.e., change the mock component CommonDropdownItemsMobileWrapper
to return p.isOpen ? p.children : null) so the resubmit action is actually
behind the open/close transition and the test enforces the production behavior
in MemesLeaderboardDrop.
In `@__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx`:
- Around line 101-112: The test is brittle because it spies on React.useState;
remove the useState spy in the "closes fullscreen when button clicked" test and
instead exercise the component through user interactions: render
MemesSingleWaveDropInfoPanel, simulate the user clicking the "Enter fullscreen
view" control (or whatever affordance opens fullscreen), then click the "Exit
fullscreen view" button via screen.getByRole and assert the visible outcome
(e.g., fullscreen element is removed, a particular aria-hidden/aria-expanded
state changes, or the Exit button is no longer in the document) rather than
asserting a mocked setState was called.
In
`@__tests__/components/waves/memes/submission/utils/resubmissionMediaType.test.ts`:
- Around line 3-31: Add unit tests covering the other branches of
getResubmissionMediaTypeInfo: add cases that pass mimeType "video/mp4" expecting
{ label: "Video", isInteractive: false }, mimeType "text/html" expecting {
label: "HTML", isInteractive: true }, an unknown mimeType like "image/png"
expecting { label: null, isInteractive: false }, and a URL with mixed case and
query/fragment like "https://x/y.GLB?v=1#frag" expecting { label: "GLB",
isInteractive: true }; place these new it(...) blocks alongside the existing
GLTF/GLB tests in the same test file to exercise extension stripping and
case-insensitive normalization in getResubmissionMediaTypeInfo.
In `@components/waves/drop/MemesDropDetailsSection.tsx`:
- Around line 39-41: Change the null-only check for artworkMedia to a defensive
truthy/nonnull check so undefined doesn’t slip through: update the
hasArtworkDownload expression (which currently uses artworkMedia !== null &&
fileInfo !== null) to ensure artworkMedia is present (e.g., artworkMedia != null
or a truthy check like Boolean(artworkMedia)) before checking fileInfo; keep
hasDownloads logic using previewImageData and promoVideoData unchanged besides
using consistent null/undefined-safe checks if desired.
In `@components/waves/drop/MemesDropFullscreenOverlay.tsx`:
- Line 18: The prop type for onClose in the MemesDropFullscreenOverlay component
is too specific; change the onClose signature from
MouseEventHandler<HTMLButtonElement> to a simple () => void in the
props/interface (reference: onClose and MemesDropFullscreenOverlay) so it can be
invoked from clicks, keyboard (ESC) or backdrop events, and remove the
now-unused MouseEventHandler import; update any callers to pass a zero-arg
function if needed.
In `@components/waves/drop/memesDropPanelTypes.ts`:
- Around line 1-14: MemesDropMedia currently defines mime_type (snake_case)
while MemesDropDownloadData and MemesDropFileInfo use camelCase (fileInfo, name,
extension); make the naming consistent by choosing one convention (e.g., convert
mime_type to mimeType or convert fileInfo to file_info), update the interfaces
(MemesDropMedia, MemesDropFileInfo, MemesDropDownloadData) accordingly, and then
update all usages of those symbols (references to mime_type/mimeType and
fileInfo/file_info) throughout the codebase to match the chosen convention so
types align with their origin (API payload vs UI model).
In `@components/waves/drop/MemesDropSummarySection.tsx`:
- Line 10: The prop type for the drop summary currently forces callers to
provide description via "readonly description: string;" even though the
component checks "{description && ...}" and treats it as optional; change the
property on the relevant type/interface to be optional (e.g. "readonly
description?: string" or "readonly description?: string | undefined") so callers
can omit it and the typing matches runtime usage; update the type declaration
where "readonly description" is defined (the interface/type used by
MemesDropSummarySection) accordingly.
In `@components/waves/memes/file-upload/components/FilePreview.tsx`:
- Around line 45-56: The isGLB detection currently checks effectiveMimeType and
file?.name extensions but misses the case where there's no file and the
resubmitted media URL ends with .glb/.gltf; update the isGLB logic in
FilePreview.tsx (symbols: effectiveMimeType, isGLB, url, file) to also inspect
the url's pathname/extension when file is undefined or file?.name is
absent—e.g., extract the extension from url (similar to the helper in
resubmissionMediaType.ts) and treat .glb/.gltf as GLB so resubmitted GLB URLs
with incorrect/blank mimeType still enter the GLB branch.
In `@components/waves/memes/submission/hooks/useArtworkSubmissionFormActions.ts`:
- Around line 48-51: The handler handleContinueFromArtwork currently calls
applyProfileDefaults again even though applyProfileDefaults is already invoked
in handleContinueFromTerms and reduceProfileDefaults is idempotent; remove the
duplicate applyProfileDefaults call from handleContinueFromArtwork so it only
dispatches SET_STEP to SubmissionStep.ADDITIONAL_INFO (leave
applyProfileDefaults in handleContinueFromTerms as the single source of default
application), referencing the functions handleContinueFromArtwork,
handleContinueFromTerms, applyProfileDefaults and reduceProfileDefaults when
making the change.
- Around line 123-134: The getSubmissionData function redundantly reassigns
traits.title and traits.description after spreading traits; remove the no-op
explicit entries and return traits directly from state.traits. Locate the
getSubmissionData useCallback (references: getSubmissionData, state, artworkUrl,
traits, operationalData) and update its return to include imageUrl: artworkUrl,
traits: { ...traits }, operationalData (or simply traits), eliminating title:
traits.title and description: traits.description.
In
`@components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts`:
- Around line 91-110: The function setArtworkUploaded in
useArtworkSubmissionMediaControls is misleading because setArtworkUploaded(true)
is a no-op while false clears state; rename it to resetUploadedArtwork or
clearArtwork (and update all callers) to reflect that it clears upload/external
state, or alternatively add an inline comment above setArtworkUploaded
documenting that true intentionally no-ops and only false triggers reset; when
renaming, update the function reference in the dependency array and replace
usages that call setArtworkUploaded(true)/false, and ensure the implementation
still calls updateExternalMediaState("", state.externalMedia.provider) for
mediaSource === "url" and dispatch({ type: "RESET_UPLOAD_MEDIA" }) otherwise.
In `@components/waves/memes/submission/hooks/useArtworkSubmissionMutation.ts`:
- Around line 25-43: Update the ArtworkSubmissionData type so
externalMedia.mimeType stays narrowed to InteractiveMediaMimeType while
existingMedia.mimeType is a plain string: change externalMedia?: { url: string;
mimeType: InteractiveMediaMimeType } | undefined and existingMedia?: { url:
string; mimeType: string } | undefined; leave all other fields and the
normalized existingUrl logic untouched so callers that require interactive-only
mime types still get type-level guarantees while resubmission passthroughs
accept arbitrary existingMedia mime strings.
In `@components/waves/memes/submission/hooks/useResubmissionDelete.ts`:
- Around line 59-63: In useResubmissionDelete change the order around the auth
call and loading flag: call requestAuth() and await its result before calling
setIsDeletingOriginal(true) so the UI loading state is only set if auth
succeeds; specifically, move the requestAuth() invocation earlier (or
conditionalize setIsDeletingOriginal) around the existing requestAuth /
setIsDeletingOriginal lines to avoid the brief flash when requestAuth is
cancelled.
In `@components/waves/memes/submission/MemesArtResubmitAction.tsx`:
- Around line 115-131: The button currently uses aria-disabled but doesn't
associate the visible disabled reason with the control; create a stable id for
the disabled message (e.g., tooltipId) and render that id on the disabledReason
paragraph in the menu/button variants, then set aria-describedby={tooltipId} on
the interactive element when disabled is true; keep the existing openModal guard
(if (disabled) return) and preserve onOpenModal logic so keyboard activation
still short-circuits safely while assistive tech can announce the reason via
aria-describedby.
In `@components/waves/memes/submission/MemesArtSubmissionContainer.tsx`:
- Around line 241-302: shellDescription is only set for the form path (when
isResubmission && hasAcknowledgedResubmission && !replacementDrop), so
ResubmitAcknowledgement and ResubmitDeleteConfirmation render without shell
description; if you want consistent header subtext either compute
shellDescription to cover those substeps (e.g., branch on isResubmission +
specific substep flags) or have ResubmitAcknowledgement and
ResubmitDeleteConfirmation accept a description prop and pass it through when
rendering inside MemesArtSubmissionShell; update the logic around
shellDescription (and the render of ResubmitAcknowledgement /
ResubmitDeleteConfirmation) in MemesArtSubmissionContainer to supply the desired
text.
In `@components/waves/memes/submission/MemesArtSubmissionShell.tsx`:
- Around line 27-39: The motion wrappers around the header and close button in
MemesArtSubmissionShell (motion.h3 and motion.button) provide no animation props
and should be replaced with plain h3 and button elements (preserve title,
className, aria-label, onClose handler and inner FontAwesomeIcon) to avoid
framer-motion overhead; after replacing, remove any unused framer-motion imports
(e.g., motion) from the file to clean up unused dependencies.
In `@components/waves/memes/submission/MemesArtSubmissionStepContent.tsx`:
- Around line 60-84: mediaTypeInfo is recomputed on every render causing a new
object identity each time; wrap its computation in React.useMemo to memoize the
returned object and stabilize downstream props (use the same logic inside the
memo). Use form.mediaSource, form.selectedFile, form.existingMedia,
form.isExternalMediaValid, form.externalMediaMimeType and form.externalMediaUrl
as the memo dependency list so getResubmissionMediaTypeInfo is only recalculated
when inputs change, and leave previewRequiredMediaType, requiresPreviewImage and
requiresPromoVideoOption derived from the memoized mediaTypeInfo as before.
In `@components/waves/memes/submission/utils/submissionDraft.ts`:
- Around line 88-117: The code currently sets title/description inside the
fieldDefinitions.forEach loop from metadata.get(...) and then unconditionally
reassigns traits.title and traits.description from metadata.get(...) ??
drop.title ?? ... which is redundant and obscures the intent; update the
fieldDefinitions processing in submissionDraft.ts to skip the "title" and
"description" fields (check definition.field === "title" || "description") so
they are only resolved by the explicit fallback assignment, or alternatively add
a short inline comment above the final assignments (traits.title /
traits.description) explaining that title/description are intentionally handled
after the loop to allow drop.title and drop.parts fallback when metadata is
absent; reference the loop over fieldDefinitions, metadata.get, and the final
traits.title/traits.description assignments when making the change.
- Around line 64-72: The current booleanValue function coerces any non-true-like
string to false; change booleanValue to accept a fallback parameter (like
numberValue) and return that fallback for ambiguous inputs: normalize the input
string, explicitly check for true-like tokens ("true","1","yes","on") to return
true, check for false-like tokens ("false","0","no","off") to return false, and
otherwise return the provided fallback; update all call sites to pass the
appropriate trait default or leave a safe default when calling booleanValue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fae2a9c7-b6d7-4ecc-b771-efa4a57ad377
📒 Files selected for processing (40)
__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx__tests__/components/utils/select/dropdown/CommonDropdownItemsMobileWrapper.test.tsx__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx__tests__/components/waves/memes/submission/MemesArtResubmitAction.test.tsx__tests__/components/waves/memes/submission/MemesArtSubmissionContainer.test.tsx__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionForm.test.ts__tests__/components/waves/memes/submission/utils/resubmissionMediaType.test.ts__tests__/components/waves/memes/submission/utils/submissionDraft.test.tscomponents/memes/drops/MemesLeaderboardDrop.tsxcomponents/utils/select/dropdown/CommonDropdownItemsMobileWrapper.tsxcomponents/waves/drop/MemesDropArtworkHero.tsxcomponents/waves/drop/MemesDropDetailsSection.tsxcomponents/waves/drop/MemesDropFullscreenOverlay.tsxcomponents/waves/drop/MemesDropSummarySection.tsxcomponents/waves/drop/MemesSingleWaveDrop.tsxcomponents/waves/drop/MemesSingleWaveDropInfoPanel.tsxcomponents/waves/drop/memesDropPanelTypes.tscomponents/waves/leaderboard/drops/WaveLeaderboardDrop.tsxcomponents/waves/memes/MemesArtSubmissionFile.tsxcomponents/waves/memes/MemesArtSubmissionModal.tsxcomponents/waves/memes/file-upload/components/FilePreview.tsxcomponents/waves/memes/file-upload/reducers/types.tscomponents/waves/memes/submission/MemesArtResubmitAction.tsxcomponents/waves/memes/submission/MemesArtSubmissionContainer.tsxcomponents/waves/memes/submission/MemesArtSubmissionShell.tsxcomponents/waves/memes/submission/MemesArtSubmissionStepContent.tsxcomponents/waves/memes/submission/ResubmitAcknowledgement.tsxcomponents/waves/memes/submission/ResubmitDeleteConfirmation.tsxcomponents/waves/memes/submission/hooks/artworkSubmissionFormState.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionForm.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionFormActions.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionMutation.tscomponents/waves/memes/submission/hooks/useResubmissionDelete.tscomponents/waves/memes/submission/preview/MemesSubmissionPreviewScreen.tsxcomponents/waves/memes/submission/steps/AdditionalInfoStep.tsxcomponents/waves/memes/submission/steps/ArtworkStep.tsxcomponents/waves/memes/submission/utils/buildPreviewDrop.tscomponents/waves/memes/submission/utils/resubmissionMediaType.tscomponents/waves/memes/submission/utils/submissionDraft.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionForm.test.ts (1)
223-223: Nit: restoreglobal.FileReaderacross tests.Several tests (lines 223, 264, 327, 355, 396, 531) assign
global.FileReader = MockFileReader as anybut never restore it. Because each consumer re-assigns it, this works today, but any future test that relies on the real jsdomFileReader(or runs after these without re-assigning) would inherit the last mock. Consider capturing and restoring in anafterEachto keep tests hermetic.♻️ Suggested harness tweak
+ const OriginalFileReader = global.FileReader; afterEach(() => { jest.clearAllMocks(); + global.FileReader = OriginalFileReader; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionForm.test.ts` at line 223, Tests assign global.FileReader = MockFileReader but never restore it; capture the original global.FileReader before overriding (e.g., const originalFileReader = global.FileReader) and restore it in an afterEach hook so each test resets the environment. Update the file to save originalFileReader, assign global.FileReader = MockFileReader in the individual tests or a beforeEach, and ensure afterEach sets global.FileReader = originalFileReader (and deletes it if it was undefined) to keep tests hermetic; reference MockFileReader, global.FileReader, and add an afterEach to perform the restore.components/waves/memes/submission/hooks/useArtworkSubmissionForm.ts (1)
27-82: LGTM — clean decomposition and react-query wiring.The split between
profileIdentityDefaults(used for lazy reducer init) andprofileDefaults(passed to form actions for deferred bio hydration) is consistent with the test that manualabout_artistis not overwritten after the CIC fetch resolves. Query is correctly gated by!isDraftInitialized && normalizedProfileHandle.length > 0so a draft-initialized flow makes no network request.Minor nit: the
initialStateInputmemo on lines 62–68 is used only as the third argument touseReducer, whose lazy initializer runs exactly once at mount, so the memoization provides no benefit — a plain object literal would be equivalent. Feel free to leave as-is for consistency with the other memos.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionForm.ts` around lines 27 - 82, The useMemo for initialStateInput in useArtworkSubmissionForm is unnecessary because it's only used as the lazy initializer argument to useReducer (createInitialState runs once at mount); replace the useMemo-created value with a plain object literal passed directly as the third argument to useReducer (i.e., eliminate the initialStateInput constant and pass { initialDraft, profileDefaults: profileIdentityDefaults } into useReducer(createInitialState)), keeping references to formReducer and createInitialState intact.components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts (1)
128-147: RenamesetArtworkUploadedto clarify its asymmetric behavior.The function only performs work when called with
falseand silently returns when called withtrue. This asymmetry contradicts the naming convention of a boolean setter, which typically implies symmetric behavior. Callers expectingsetArtworkUploaded(true)to mark an upload as complete will be silently ignored.Consider renaming to
clearArtworkUpload()(parameterless) orresetArtworkUpload()to accurately convey that this function only resets state. Update the call site atuseArtworkSubmissionForm.tsline 106 accordingly.Suggested refactor
- const setArtworkUploaded = useCallback( - (uploaded: boolean) => { - if (uploaded) { - return; - } - - if (state.mediaSource === "url") { - updateExternalMediaState("", state.externalMedia.provider); - return; - } - - dispatch({ type: "RESET_UPLOAD_MEDIA" }); - }, + const clearArtworkUpload = useCallback(() => { + if (state.mediaSource === "url") { + updateExternalMediaState("", state.externalMedia.provider); + return; + } + dispatch({ type: "RESET_UPLOAD_MEDIA" }); + }, [ dispatch, state.externalMedia.provider, state.mediaSource, updateExternalMediaState, ] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts` around lines 128 - 147, The helper setArtworkUploaded has asymmetric behavior (it only acts when called with false) and should be renamed to convey that it resets/clears upload state; rename setArtworkUploaded to a parameterless name like clearArtworkUpload or resetArtworkUpload, remove the boolean parameter, and update its implementation to always perform the reset logic (handle state.mediaSource === "url" by calling updateExternalMediaState("", state.externalMedia.provider) and otherwise dispatch({ type: "RESET_UPLOAD_MEDIA" })); then update all call sites (notably the caller in useArtworkSubmissionForm where setArtworkUploaded(...) is invoked) to call the new clear/reset function with no arguments.
🤖 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/memes/drops/MemesLeaderboardDrop.tsx`:
- Around line 73-85: The openResubmitAfterMenuClose flag can persist and trigger
the resubmit modal unexpectedly; update the component to defensively clear that
flag when the menu becomes active again or on unmount: add a useEffect that
watches isActive and if isActive === true calls
setOpenResubmitAfterMenuClose(false), and ensure the effect returns a cleanup
that also clears setOpenResubmitAfterMenuClose(false) to cover unmounts; this
complements the existing openResubmitAfterMobileMenuCloses and
handleMobileMenuAfterLeave logic (also referencing setIsActive and
CommonDropdownItemsMobileWrapper) so the deferred-open flag never survives a
new/opened menu or component teardown.
In `@components/waves/drop/MemesSingleWaveDropInfoPanel.tsx`:
- Around line 99-149: The component defines isFullscreen and only a
handleCloseFullscreen (which sets isFullscreen false) while passing isFullscreen
to MemesDropFullscreenOverlay, so the overlay can never open; either remove the
unused fullscreen state/overlay wiring (delete isFullscreen,
handleCloseFullscreen, and MemesDropFullscreenOverlay) or implement and wire an
open handler: add handleOpenFullscreen that calls setIsFullscreen(true) and pass
it as a prop (e.g., onOpenFullscreen) into MemesDropArtworkHero so that the hero
can trigger fullscreen; update any prop names consistently between
MemesSingleWaveDropInfoPanel (isFullscreen, handleOpenFullscreen,
handleCloseFullscreen) and the child component to ensure the overlay can
actually be opened.
---
Nitpick comments:
In
`@__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionForm.test.ts`:
- Line 223: Tests assign global.FileReader = MockFileReader but never restore
it; capture the original global.FileReader before overriding (e.g., const
originalFileReader = global.FileReader) and restore it in an afterEach hook so
each test resets the environment. Update the file to save originalFileReader,
assign global.FileReader = MockFileReader in the individual tests or a
beforeEach, and ensure afterEach sets global.FileReader = originalFileReader
(and deletes it if it was undefined) to keep tests hermetic; reference
MockFileReader, global.FileReader, and add an afterEach to perform the restore.
In `@components/waves/memes/submission/hooks/useArtworkSubmissionForm.ts`:
- Around line 27-82: The useMemo for initialStateInput in
useArtworkSubmissionForm is unnecessary because it's only used as the lazy
initializer argument to useReducer (createInitialState runs once at mount);
replace the useMemo-created value with a plain object literal passed directly as
the third argument to useReducer (i.e., eliminate the initialStateInput constant
and pass { initialDraft, profileDefaults: profileIdentityDefaults } into
useReducer(createInitialState)), keeping references to formReducer and
createInitialState intact.
In
`@components/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.ts`:
- Around line 128-147: The helper setArtworkUploaded has asymmetric behavior (it
only acts when called with false) and should be renamed to convey that it
resets/clears upload state; rename setArtworkUploaded to a parameterless name
like clearArtworkUpload or resetArtworkUpload, remove the boolean parameter, and
update its implementation to always perform the reset logic (handle
state.mediaSource === "url" by calling updateExternalMediaState("",
state.externalMedia.provider) and otherwise dispatch({ type:
"RESET_UPLOAD_MEDIA" })); then update all call sites (notably the caller in
useArtworkSubmissionForm where setArtworkUploaded(...) is invoked) to call the
new clear/reset function with no arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fff94ced-8b10-41e0-bcbc-3cfe50b80c9a
📒 Files selected for processing (22)
__tests__/components/memes/drops/MemesLeaderboardDrop.test.tsx__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrop.test.tsx__tests__/components/waves/leaderboard/drops/WaveLeaderboardDrops.test.tsx__tests__/components/waves/memes/MemesArtSubmissionFile.test.tsx__tests__/components/waves/memes/submission/hooks/useArtworkSubmissionForm.test.ts__tests__/components/waves/memes/submission/steps/ArtworkStep.test.tsxcomponents/memes/drops/MemesLeaderboardDrop.tsxcomponents/waves/drop/MemesDropFullscreenOverlay.tsxcomponents/waves/drop/MemesSingleWaveDropInfoPanel.tsxcomponents/waves/leaderboard/drops/WaveLeaderboardDrop.tsxcomponents/waves/leaderboard/drops/WaveLeaderboardDrops.tsxcomponents/waves/memes/MemesArtSubmissionFile.tsxcomponents/waves/memes/file-upload/reducers/types.tscomponents/waves/memes/submission/MemesArtSubmissionShell.tsxcomponents/waves/memes/submission/MemesArtSubmissionStepContent.tsxcomponents/waves/memes/submission/hooks/artworkSubmissionFormState.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionForm.tscomponents/waves/memes/submission/hooks/useArtworkSubmissionMediaControls.tscomponents/waves/memes/submission/hooks/useResubmissionDelete.tscomponents/waves/memes/submission/steps/ArtworkStep.tsxcomponents/waves/memes/submission/utils/buildPreviewDrop.ts
✅ Files skipped from review due to trivial changes (1)
- tests/components/waves/memes/submission/steps/ArtworkStep.test.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- components/waves/leaderboard/drops/WaveLeaderboardDrop.tsx
- components/waves/memes/submission/utils/buildPreviewDrop.ts
- components/waves/memes/MemesArtSubmissionFile.tsx
- components/waves/memes/submission/MemesArtSubmissionShell.tsx
- tests/components/memes/drops/MemesLeaderboardDrop.test.tsx
- tests/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx
- components/waves/memes/file-upload/reducers/types.ts
- components/waves/drop/MemesDropFullscreenOverlay.tsx
- components/waves/memes/submission/hooks/useResubmissionDelete.ts
- components/waves/memes/submission/MemesArtSubmissionStepContent.tsx
- components/waves/memes/submission/hooks/artworkSubmissionFormState.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/waves/drop/MemesDropFullscreenOverlay.tsx (1)
143-150: Usetw-flex-1on the image container instead oftw-h-full.The
<dialog>is aflex-colwithtw-h-full tw-max-h-full, and this image wrapper also claimstw-h-fullalongside the (auto-sized) header row. In a column flex container,h-fullon a child resolves to 100% of the parent height, so the two children's intrinsic heights can sum to more than the dialog height, which may push the image out of the viewport or rely on flex shrinking to hide the overflow. Usingtw-flex-1 tw-min-h-0is the idiomatic pattern that reserves exactly the remaining vertical space for the image while keepingtw-object-containworking correctly.♻️ Proposed fix
- <div className="tw-flex tw-h-full tw-w-full tw-items-center tw-justify-center"> + <div className="tw-flex tw-min-h-0 tw-w-full tw-flex-1 tw-items-center tw-justify-center">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drop/MemesDropFullscreenOverlay.tsx` around lines 143 - 150, The image container div currently uses "tw-h-full" which in a column flex parent can exceed available space; update the wrapper (the div that wraps the <img> in MemesDropFullscreenOverlay) to use "tw-flex-1 tw-min-h-0" instead of "tw-h-full" so the container takes only the remaining flex space and allows the <img> (with tw-object-contain) to size correctly without overflowing the dialog.
🤖 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/drop/MemesDropFullscreenOverlay.tsx`:
- Around line 104-107: The code currently coerces a missing drop.rank into 1
when rendering SingleWaveDropPosition, which misrepresents unranked drops;
update the prop passed to SingleWaveDropPosition to forward the actual nullable
value (e.g., pass drop.rank as-is or drop.rank ?? null) so the component
receives number | null and can render the neutral/unranked state; ensure no
fallback to 1 remains and adjust any surrounding logic that assumed a non-null
rank on SingleWaveDropPosition.
---
Nitpick comments:
In `@components/waves/drop/MemesDropFullscreenOverlay.tsx`:
- Around line 143-150: The image container div currently uses "tw-h-full" which
in a column flex parent can exceed available space; update the wrapper (the div
that wraps the <img> in MemesDropFullscreenOverlay) to use "tw-flex-1
tw-min-h-0" instead of "tw-h-full" so the container takes only the remaining
flex space and allows the <img> (with tw-object-contain) to size correctly
without overflowing the dialog.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 782ca3fe-dc25-49f9-9981-e67d0975b9fd
📒 Files selected for processing (6)
__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsxcomponents/memes/drops/MemesLeaderboardDrop.tsxcomponents/waves/drop/MemesDropArtworkHero.tsxcomponents/waves/drop/MemesDropFullscreenOverlay.tsxcomponents/waves/drop/MemesSingleWaveDropInfoPanel.tsxcomponents/waves/drop/memesDropPanelTypes.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- components/waves/drop/MemesDropArtworkHero.tsx
- components/memes/drops/MemesLeaderboardDrop.tsx
- components/waves/drop/memesDropPanelTypes.ts
- tests/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx
|
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx (1)
167-178: Avoid coupling this test touseStatehook order.This test relies on the first
React.useStatecall belonging to fullscreen state. Adding another state hook before it would break the test for the wrong reason. The component already has two state hooks (line 31-32), making this fragile. Use the existing "Open fullscreen view" button instead to test the actual UI flow.🧪 Proposed test rewrite
it("closes fullscreen when button clicked", async () => { - const setState = jest.fn(); - const spy = jest - .spyOn(React, "useState") - .mockImplementationOnce(() => [true, setState]); render(<MemesSingleWaveDropInfoPanel drop={baseDrop} wave={null} />); + + await userEvent.click( + screen.getByRole("button", { name: "Open fullscreen view" }) + ); + + expect( + screen.getByRole("button", { name: "Exit fullscreen view" }) + ).toBeInTheDocument(); + await userEvent.click( screen.getByRole("button", { name: "Exit fullscreen view" }) ); - expect(setState).toHaveBeenCalledWith(false); - spy.mockRestore(); + + expect( + screen.queryByRole("button", { name: "Exit fullscreen view" }) + ).not.toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx` around lines 167 - 178, The test "closes fullscreen when button clicked" is fragile because it spies on React.useState and assumes the first hook is the fullscreen state; remove the useState spy and instead drive the UI: click the existing "Open fullscreen view" button to open fullscreen in the rendered MemesSingleWaveDropInfoPanel, then click the "Exit fullscreen view" button and assert the fullscreen UI is dismissed (e.g., the fullscreen element or exit button is no longer in the document). Stop using the setState mock and React.useState spy; rely on userEvent interactions with the "Open fullscreen view" and "Exit fullscreen view" buttons to verify behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsx`:
- Around line 167-178: The test "closes fullscreen when button clicked" is
fragile because it spies on React.useState and assumes the first hook is the
fullscreen state; remove the useState spy and instead drive the UI: click the
existing "Open fullscreen view" button to open fullscreen in the rendered
MemesSingleWaveDropInfoPanel, then click the "Exit fullscreen view" button and
assert the fullscreen UI is dismissed (e.g., the fullscreen element or exit
button is no longer in the document). Stop using the setState mock and
React.useState spy; rely on userEvent interactions with the "Open fullscreen
view" and "Exit fullscreen view" buttons to verify behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9275fa07-9ca2-4619-a7fc-4ccf8885ff8b
📒 Files selected for processing (2)
__tests__/components/waves/drop/MemesSingleWaveDropInfoPanel.test.tsxcomponents/waves/drop/MemesDropFullscreenOverlay.tsx
ragnep
approved these changes
Apr 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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests