Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (5)
You can disable this status message by setting the WalkthroughRefactors NFT image loading logic; makes MemePage tabs dynamic with URL-driven focus and cancellable parallel fetches; guards activity fetches when hidden; groups NFTs by meme and improves infinite scroll; moves Tweet embeds to client-side dynamic loading; updates tests and many pages to add rel/security and remove explicit viewport tags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant R as Router
participant MP as MemePage
participant API as Backend
participant Art as MemePageArt (dynamic)
participant Act as MemePageActivity (dynamic)
participant TL as MemePageTimeline (dynamic)
U->>MP: Open /memes/[id]?focus=art
MP->>R: read pathname + searchParams (focus)
note right of MP: activeTab = focus || default
par Parallel fetch
MP->>API: GET memes_extended_data
MP->>API: GET nft
end
API-->>MP: responses
MP->>R: replace(focus=<tab>, {scroll:false})
MP->>Art: dynamic import when art tab loaded
MP->>Act: dynamic import when activity tab loaded
MP->>TL: dynamic import when timeline tab loaded
note over MP: render guarded by loadedTabs and nft presence
alt Tab switch
U->>MP: Click Activity tab
MP->>R: replace focus=activity (scroll:false)
MP->>Act: dynamic import if needed
end
sequenceDiagram
autonumber
participant Act as MemePageActivity
participant API as Backend
Act->>Act: useEffect(show,nft,...)
alt show=false or nft undefined
Act-->>API: (no request)
note right of Act: early exit
else
Act->>API: fetch activity
API-->>Act: data
end
sequenceDiagram
autonumber
participant IR as NFTImageRenderer
participant Img as Next/Image
IR->>IR: compute shouldLazyLoad = thumbnail || height <= 300
alt shouldLazyLoad
IR->>Img: loading=lazy, priority=false, fetchPriority=auto
else
IR->>Img: loading=eager, priority=true, fetchPriority=high
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/the-memes/TheMemes.tsx (1)
270-275: Harden fetch path: handle errors and use functional state update.If
fetchUrlrejects,fetchingmay stay true, leaving the UI stuck. Also prefer functionalsetNftsto avoid stale captures.- fetchUrl(nftsNextPage).then((responseNfts: DBResponse) => { - setNfts([...nfts, ...responseNfts.data]); - setNftsNextPage(responseNfts.next); - setFetching(false); - }); + fetchUrl(nftsNextPage) + .then((responseNfts: DBResponse) => { + setNfts((prev) => [...prev, ...(responseNfts.data ?? [])]); + setNftsNextPage(responseNfts.next); + }) + .catch(() => { + // optionally surface a toast/log here + }) + .finally(() => setFetching(false));components/the-memes/MemePageActivity.tsx (2)
29-62: Prevent setState after unmount and add error handling in effect.
isMountedis declared but unused; state updates may occur after unmount whenshowflips or the component unmounts. Add a cancellation flag (and optionally abort) plus.catch/.finally.- useEffect(() => { - if (!props.show || !props.nft) { - return; - } - let isMounted = true; + useEffect(() => { + if (!props.show || !props.nft) { + setActivityTotalResults(0); + setActivity([]); + return; + } + let cancelled = false; if (props.nft) { let url = `${publicEnv.API_ENDPOINT}/api/transactions?contract=${MEMES_CONTRACT}&id=${props.nft.id}&page_size=${props.pageSize}&page=${activityPage}`; switch (activityTypeFilter) { case TypeFilter.SALES: url += `&filter=sales`; break; case TypeFilter.TRANSFERS: url += `&filter=transfers`; break; case TypeFilter.AIRDROPS: url += `&filter=airdrops`; break; case TypeFilter.MINTS: url += `&filter=mints`; break; case TypeFilter.BURNS: url += `&filter=burns`; break; } - fetchUrl(url).then((response: DBResponse) => { - setActivityTotalResults(response.count); - setActivity(response.data); - }); + fetchUrl(url) + .then((response: DBResponse) => { + if (cancelled) return; + setActivityTotalResults(response.count ?? 0); + setActivity(response.data ?? []); + }) + .catch(() => { + if (cancelled) return; + setActivityTotalResults(0); + setActivity([]); + }); } return () => { - isMounted = false; + cancelled = true; }; - }, [props.show, props.nft, props.pageSize, activityPage, activityTypeFilter]); + }, [props.show, props.nft?.id, props.pageSize, activityPage, activityTypeFilter]);As per coding guidelines
17-21: Type polish: wrap props in Readonly<{}>Wrap the inline props definition in
Readonly<{…}>to align with existing TS patterns used throughout the codebase.-export function MemePageActivity(props: { +export function MemePageActivity(props: Readonly<{ show: boolean; nft: NFT | undefined; pageSize: number; -}) { +}>) {
🧹 Nitpick comments (12)
components/nft-image/renderers/NFTImageRenderer.tsx (1)
32-37: Constrainpriorityusage to true LCP images to avoid excessive preloads.Setting both
loading="eager"andpriorityon all non-grid images can over-eagerly preload if multiple are on screen. Consider gatingpriorityto at most the first/visible image (or pass a dedicatedisLCPprop) and let others stay eager withoutpriority, or rely solely onpriorityand omitloading. Also confirm this aligns with Next Image guidance for usingprioritysparingly.Would you like me to scan the codebase to locate all call sites and suggest a minimal
isLCPprop threading?components/the-memes/TheMemes.tsx (3)
227-263: Avoid duplicating derived state; compute groups withuseMemo.
nftMemesandnftsByMemeare purely derived fromnfts+sortDir. Storing them in state risks divergence and extra renders. Prefer a singleuseMemothat returns{ nftMemes, nftsByMeme }fromnftsandsortDir.
291-326: Infinite scroll: consider IntersectionObserver over throttled scroll.The throttle works, but an IntersectionObserver on a sentinel is simpler, more performant, and avoids main-thread scroll work. It also plays nicer with mobile browsers’ scroll behavior.
104-106: Remove unused consolidation key state (or wire it in).
connectedConsolidationKeyis set but not used. Drop it or apply it where intended to avoid dead state.Also applies to: 327-333
components/the-memes/MemePage.tsx (6)
138-177: Parallel fetch + unmount guard is good; consider react‑query and abortsNice switch to
Promise.allwith a cancel flag. For better caching, deduping, retries, and automatic cancellation, migrate these fetches to react‑query (useQueries/useQuery) as per guidelines. If you keep manual fetches, addAbortControllerso the requests themselves are aborted on unmount/id change.Example (outline, not drop‑in):
const [{ data: meta }, { data: nftData }] = useQueries({ queries: [ { queryKey: ['memes_meta', nftId], queryFn: () => fetchUrl(`${publicEnv.API_ENDPOINT}/api/memes_extended_data?id=${nftId}`) }, { queryKey: ['nft', nftId], queryFn: () => fetchUrl(`${publicEnv.API_ENDPOINT}/api/nfts?id=${nftId}&contract=${MEMES_CONTRACT}`) }, ], });
179-191: Use iteration intended for side‑effects
mapis used for side‑effects; preferfor...of/forEachorreducefor clarity and to avoid accidental misuse.- data.map((d: Transaction) => { + for (const d of data) { if (connectedWallets.some((w) => areEqualAddresses(w, d.from_address))) { countOut += d.token_count; } if (connectedWallets.some((w) => areEqualAddresses(w, d.to_address))) { countIn += d.token_count; } - }); + }
193-211: Guard async state updates and handle errors in transactions fetchAdd a cancel flag (or
AbortController) so late responses don’t set state after unmount/change. Also setuserLoaded(true)on catch to avoid indefinite loading states.- useEffect(() => { - if (connectedWallets.length && nftId) { - fetchUrl( + useEffect(() => { + let cancelled = false; + if (connectedWallets.length && nftId) { + const url = `${publicEnv.API_ENDPOINT}/api/transactions?contract=${MEMES_CONTRACT}&wallet=${connectedWallets.join(",")}&id=${nftId}`; - ).then((response: DBResponse) => { - setTransactions(response.data); - updateNftBalances(response.data); - setUserLoaded(true); - }); + fetchUrl(url) + .then((response: DBResponse) => { + if (cancelled) return; + setTransactions(response.data); + updateNftBalances(response.data); + setUserLoaded(true); + }) + .catch(() => { + if (cancelled) return; + setTransactions([]); + setNftBalance(0); + setUserLoaded(true); + }); } else { setNftBalance(0); setUserLoaded(true); setTransactions([]); } - }, [nftId, connectedWallets]); + return () => { + cancelled = true; + }; + }, [nftId, connectedWallets]);
213-223: Also cancel/guard the TDH consolidation fetch; specify radixPrevent setState after unmount and make
parseIntexplicit.- useEffect(() => { - if (connectedWallets.length > 0 && nftId) { - commonApiFetch<ConsolidatedTDH>({ + useEffect(() => { + let cancelled = false; + if (connectedWallets.length > 0 && nftId) { + commonApiFetch<ConsolidatedTDH>({ endpoint: `tdh/consolidation/${connectedProfile?.consolidation_key}`, }).then((response) => { - setMyOwner(response); - setMyTDH(response.memes.find((m) => m.id === parseInt(nftId))); - setMyRank(response.memes_ranks.find((m) => m.id === parseInt(nftId))); + if (cancelled) return; + setMyOwner(response); + setMyTDH(response.memes.find((m) => m.id === parseInt(nftId, 10))); + setMyRank(response.memes_ranks.find((m) => m.id === parseInt(nftId, 10))); }); } - }, [nftId, connectedWallets]); + return () => { + cancelled = true; + }; + }, [nftId, connectedWallets]);
366-375: Optional: pre‑warm next tab for snappier UXYou can pre‑warm a tab’s chunk on hover (e.g., set its
loadedTabs[focus]=trueon mouse enter) to eliminate the first‑click latency.
41-51: Add loading fallbacks to each dynamic import
All three modules render anywindow/documentcalls inside event handlers—no module-scope globals—so SSR remains safe (nossr: falseneeded).Example fallback wiring:
-const MemePageArt = dynamic(() => - import("./MemePageArt").then((mod) => mod.MemePageArt) -); +const MemePageArt = dynamic( + () => import("./MemePageArt").then((mod) => mod.MemePageArt), + { loading: () => null } +);Repeat for
MemePageActivityandMemePageTimelineas desired.__tests__/components/the-memes/MemePage.test.tsx (2)
248-278: Tab interaction coverage is strong; consider asserting initial replaceYou thoroughly assert tab updates and URL sync. Optionally, add an assertion that on first mount (no focus) we
replace(...?focus=live, {scroll:false})once.Also applies to: 281-327
342-350: Test name no longer matches behaviorFetching is now parallel; rename to avoid implying sequencing.
-it("fetches NFT data after metadata loads", async () => { +it("fetches NFT metadata and NFT in parallel", async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx(2 hunks)__tests__/components/the-memes/MemePage.test.tsx(11 hunks)__tests__/components/the-memes/MemePageActivity.test.tsx(2 hunks)components/nft-image/renderers/NFTImageRenderer.tsx(1 hunks)components/the-memes/MemePage.tsx(6 hunks)components/the-memes/MemePageActivity.tsx(3 hunks)components/the-memes/TheMemes.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript for implementation code
Files:
__tests__/components/the-memes/MemePageActivity.test.tsxcomponents/the-memes/TheMemes.tsxcomponents/the-memes/MemePage.tsx__tests__/components/the-memes/MemePage.test.tsxcomponents/the-memes/MemePageActivity.tsxcomponents/nft-image/renderers/NFTImageRenderer.tsx__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks
Files:
__tests__/components/the-memes/MemePageActivity.test.tsxcomponents/the-memes/TheMemes.tsxcomponents/the-memes/MemePage.tsx__tests__/components/the-memes/MemePage.test.tsxcomponents/the-memes/MemePageActivity.tsxcomponents/nft-image/renderers/NFTImageRenderer.tsx__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx
**/{__tests__/**/*.{ts,tsx},*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.{ts,tsx},*.test.tsx}: Place tests in tests directories or alongside components as ComponentName.test.tsx
Mock external dependencies and APIs in tests
Files:
__tests__/components/the-memes/MemePageActivity.test.tsx__tests__/components/the-memes/MemePage.test.tsx__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/the-memes/MemePageActivity.test.tsx__tests__/components/the-memes/MemePage.test.tsx__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/the-memes/MemePageActivity.test.tsx__tests__/components/the-memes/MemePage.test.tsx__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-28T12:32:36.068Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: AGENTS.md:0-0
Timestamp: 2025-09-28T12:32:36.068Z
Learning: Applies to **/{__tests__/**/*.{ts,tsx},*.test.tsx} : Mock external dependencies and APIs in tests
Applied to files:
__tests__/components/the-memes/MemePage.test.tsx
🧬 Code graph analysis (5)
__tests__/components/the-memes/MemePageActivity.test.tsx (1)
components/the-memes/MemePageActivity.tsx (1)
MemePageActivity(17-194)
components/the-memes/TheMemes.tsx (1)
entities/INFT.ts (1)
NFTWithMemesExtendedData(84-84)
components/the-memes/MemePage.tsx (8)
components/the-memes/MemePageArt.tsx (1)
MemePageArt(26-442)components/the-memes/MemePageActivity.tsx (1)
MemePageActivity(17-194)components/the-memes/MemePageTimeline.tsx (1)
MemePageTimeline(11-42)contexts/TitleContext.tsx (1)
useTitle(188-194)components/auth/Auth.tsx (1)
AuthContext(83-93)services/6529api.ts (1)
fetchUrl(6-19)constants.ts (1)
MEMES_CONTRACT(6-6)entities/IDBResponse.ts (1)
DBResponse(1-6)
__tests__/components/the-memes/MemePage.test.tsx (1)
components/auth/Auth.tsx (1)
AuthContext(83-93)
__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx (1)
components/nft-image/renderers/NFTImageRenderer.tsx (1)
NFTImageRenderer(23-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
components/nft-image/renderers/NFTImageRenderer.tsx (1)
25-26: LGTM on lazy/eager heuristic.Using thumbnail or a 300px height as the lazy criterion reads well and matches grid vs. feature usage.
__tests__/components/nft-image/renderers/NFTImageRenderer.test.tsx (1)
407-415: Tests correctly assert lazy for grid and eager/high for feature images.The mocked attributes validate the new loading/priority/fetchPriority semantics and give good coverage for both cases.
Also applies to: 417-423
__tests__/components/the-memes/MemePageActivity.test.tsx (2)
13-17: Named mock improves test readability.Setting a displayName on the row mock is a nice touch for debugging.
106-111: Good guard coverage.The “skips fetching when tab is hidden” test matches the new early-return logic.
components/the-memes/MemePage.tsx (3)
63-76: URL‑driven focus and tab‑load gating look solidGood use of search params -> state, prevention of loops, and
router.replace({ scroll: false }). TheloadedTabsguard avoids mounting heavy tabs until first visit. LGTM.Also applies to: 92-105, 111-121
134-137: Title effect dependency is correctIncluding
setTitlein deps avoids stale closures. LGTM.
303-322: Deferred render for heavy tabs is on pointConditioning Art/Activity/Timeline on
loadedTabs[...]avoids unnecessary mounts and fetches. Nicely done.__tests__/components/the-memes/MemePage.test.tsx (5)
10-14: Router/search‑params mocks look correct and stableGood addition of
usePathnameand a controlledreplacecapturingfocus. This mirrors the production behavior ofrouter.replace(url, { scroll: false }). LGTM.Also applies to: 84-98, 100-110
66-76: Component mocks keep tests fast/deterministicLightweight mocks for countdown, image, and navigation are appropriate and keep DOM simple. LGTM.
Based on learnings
Also applies to: 78-83
352-369: Empty metadata path assertion is validEnsuring nav isn’t rendered and only a single NFTs call is made matches the new logic. LGTM.
372-489: Wallet integration tests are preciseGreat checks for joined wallet query and for the zero‑wallets path avoiding the transactions call.
Based on learnings
491-525: Loading and content rendering tests are appropriateCovers never‑resolving fetch and post‑load assertions cleanly. LGTM.
Also applies to: 527-540
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
components/utils/CommonFilterTargetSelect.tsx (2)
41-47: Add value and read from event for form compatibility and clarityInclude value so forms serialize correctly and read the selected id from the event (avoids creating a new closure per item and matches native patterns).
<input id={inputId} name={groupName} type="radio" - checked={selected === target.id} - onChange={() => onChange(target.id)} + value={target.id} + checked={selected === target.id} + onChange={(e) => onChange(e.currentTarget.value as FilterTargetType)} className="tw-form-radio tw-h-4 tw-w-4 tw-bg-iron-700 tw-border-iron-600 tw-border tw-border-solid focus:tw-ring-2 tw-ring-offset-iron-800 tw-text-primary-400 focus:tw-ring-primary-400 tw-cursor-pointer" />
32-32: Remove redundant utilitytw-space-y-0 has no effect in a horizontal flex row; drop it to reduce CSS noise.
- <div className="tw-flex tw-items-center tw-space-x-6 tw-space-y-0"> + <div className="tw-flex tw-items-center tw-space-x-6">__tests__/components/utils/CommonFilterTargetSelect.test.tsx (5)
22-23: Scope queries to the group to avoid future collisionsQuery radios within the located group for resilience if more radios appear in the DOM.
- const buttons = screen.getAllByRole("radio"); + const buttons = within(group).getAllByRole("radio");Also add the import:
-import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react";
27-28: Assert single emissionVerify there’s exactly one change event after click to catch accidental double-fires.
expect(onChange).toHaveBeenCalledWith(FilterTargetType.OUTGOING); + expect(onChange).toHaveBeenCalledTimes(1);
44-45: Assert initial checked stateConfirm the initial selection for clarity before keyboard nav.
expect(allRadio).toHaveFocus(); + expect(allRadio).toBeChecked();
48-49: Assert single emission on ArrowRightAdd a call count assertion after keyboard interaction.
expect(onChange).toHaveBeenCalledWith(FilterTargetType.OUTGOING); + expect(onChange).toHaveBeenCalledTimes(1);
65-66: Assert single emission on ArrowLeftAfter mockClear(), enforce exactly one call.
expect(onChange).toHaveBeenCalledWith(FilterTargetType.ALL); + expect(onChange).toHaveBeenCalledTimes(1);components/drops/view/part/dropPartMarkdown/renderers.tsx (3)
15-21: Add ARIA semantics to the loading skeletonImprove accessibility by exposing loading state.
-const TweetEmbedSkeleton = () => ( - <div - data-testid="tweet-embed-loading" - className="tw-flex tw-items-center tw-justify-center tw-w-full tw-min-h-[10rem] tw-rounded-lg tw-bg-iron-900/60 tw-animate-pulse tw-text-iron-300"> +const TweetEmbedSkeleton = () => ( + <div + data-testid="tweet-embed-loading" + role="status" + aria-busy="true" + aria-live="polite" + aria-label="Loading tweet" + className="tw-flex tw-items-center tw-justify-center tw-w-full tw-min-h-[10rem] tw-rounded-lg tw-bg-iron-900/60 tw-animate-pulse tw-text-iron-300"> Loading tweet… </div> )
47-49: Accept props in TweetNotFound for stricter typingAvoid potential strictFunctionTypes mismatch by accepting (and ignoring) the props.
-const TweetNotFound: TwitterComponents["TweetNotFound"] = () => - renderFallback(); +const TweetNotFound: TwitterComponents["TweetNotFound"] = (_props) => + renderFallback();
31-37: Mark props as readonly (coding guideline)Make TweetFallback props readonly for consistency.
As per coding guidelines.
-const TweetFallback = ({ href }: { href: string }) => ( +const TweetFallback = ({ href }: { readonly href: string }) => (__tests__/components/drops/view/part/DropPartMarkdown.test.tsx (3)
156-168: Reset dynamic call log between testsEnsure isolation by clearing the shared call array and defaulting mode.
beforeEach(() => { jest.clearAllMocks(); + // Ensure isolation for next/dynamic mock between tests + dynamicCalls.length = 0; + setDynamicMode("eager"); publicEnv.BASE_ENDPOINT = originalBaseEndpoint ?? FALLBACK_BASE_ENDPOINT;
506-509: Avoid coupling to the first dynamic call in assertionsUse the latest recorded call to resist prior calls affecting this test.
- const [loader, options] = dynamicCalls[0]; + const [loader, options] = dynamicCalls.at(-1)!;
131-144: Make mocked component props readonly (tests)Align test mocks with project typing conventions.
As per coding guidelines.
- default: ({ - href, - relativeHref, - }: { - href: string; - relativeHref?: string; - }) => ( + default: ({ + href, + relativeHref, + }: { + readonly href: string; + readonly relativeHref?: string; + }) => (components/drops/view/part/DropPartMarkdown.tsx (1)
141-144: Prefer readonly on renderer propsSmall typing polish to match prop immutability convention.
As per coding guidelines.
- type MarkdownRendererProps<T extends ElementType> = - ComponentPropsWithoutRef<T> & - ExtraProps & { children?: ReactNode; className?: string }; + type MarkdownRendererProps<T extends ElementType> = + Readonly<ComponentPropsWithoutRef<T> & ExtraProps> & { + readonly children?: ReactNode; + readonly className?: string; + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
__tests__/components/drops/view/part/DropPartMarkdown.test.tsx(7 hunks)__tests__/components/utils/CommonFilterTargetSelect.test.tsx(1 hunks)app/layout.tsx(0 hunks)components/drops/view/part/DropPartMarkdown.tsx(4 hunks)components/drops/view/part/dropPartMarkdown/renderers.tsx(4 hunks)components/utils/CommonFilterTargetSelect.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- app/layout.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript for implementation code
Files:
components/drops/view/part/dropPartMarkdown/renderers.tsx__tests__/components/drops/view/part/DropPartMarkdown.test.tsxcomponents/utils/CommonFilterTargetSelect.tsx__tests__/components/utils/CommonFilterTargetSelect.test.tsxcomponents/drops/view/part/DropPartMarkdown.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks
Files:
components/drops/view/part/dropPartMarkdown/renderers.tsx__tests__/components/drops/view/part/DropPartMarkdown.test.tsxcomponents/utils/CommonFilterTargetSelect.tsx__tests__/components/utils/CommonFilterTargetSelect.test.tsxcomponents/drops/view/part/DropPartMarkdown.tsx
**/{__tests__/**/*.{ts,tsx},*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.{ts,tsx},*.test.tsx}: Place tests in tests directories or alongside components as ComponentName.test.tsx
Mock external dependencies and APIs in tests
Files:
__tests__/components/drops/view/part/DropPartMarkdown.test.tsx__tests__/components/utils/CommonFilterTargetSelect.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/drops/view/part/DropPartMarkdown.test.tsx__tests__/components/utils/CommonFilterTargetSelect.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/drops/view/part/DropPartMarkdown.test.tsx__tests__/components/utils/CommonFilterTargetSelect.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-28T12:33:30.941Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:30.941Z
Learning: Applies to __tests__/components/**/*.{ts,tsx,js,jsx} : Use `testing-library/react` and `testing-library/user-event` for React component tests
Applied to files:
__tests__/components/utils/CommonFilterTargetSelect.test.tsx
🧬 Code graph analysis (2)
components/drops/view/part/dropPartMarkdown/renderers.tsx (1)
components/waves/LinkHandlerFrame.tsx (1)
LinkHandlerFrame(12-28)
__tests__/components/utils/CommonFilterTargetSelect.test.tsx (1)
components/utils/CommonFilterTargetSelect.tsx (1)
CommonFilterTargetSelect(17-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
components/utils/CommonFilterTargetSelect.tsx (1)
24-31: Good accessibility and id strategyUsing fieldset/legend and a useId-derived, unique radio group name is solid and fixes labeling and keyboard nav.
__tests__/components/utils/CommonFilterTargetSelect.test.tsx (1)
1-5: Test stack is correctRTL + user-event usage and absolute imports look good. As per coding guidelines.
Signed-off-by: prxt6529 <prxt@6529.io>
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
app/author/nft6529/page.tsx (2)
59-65: Remove dangerouslySetInnerHTML style blocksInline CSS via dangerouslySetInnerHTML violates lint rules and increases XSS risk. Move these styles to Tailwind classes or a static CSS file imported at the layout level.
Also applies to: 74-80, 95-101, 112-118, 197-204
43-43: og:url has no contentThe og:url meta tag is missing a content value. Set an absolute URL or remove it.
app/category/blog/page.tsx (3)
67-73: Inline style via dangerouslySetInnerHTMLReplace with Tailwind or static CSS. Current code trips security lint rules.
Also applies to: 81-88, 103-110, 112-118, 120-126, 206-213
49-51: Wrong og:url on category pageog:url points to a specific blog post; it should be the category URL. Fix or remove.
-<meta - property="og:url" - content="/blog/disney-deekay-their-secret-to-animation/" -/> +<meta property="og:url" content="/category/blog/" />
282-295: Revisit image fetchPriorityOn listing cards, keep loading="lazy" and drop fetchPriority unless the image is LCP.
- decoding="async" - fetchPriority="high" + decoding="async"app/museum/6529-fund-szn1/construction-token/page.tsx (1)
4-226: Move metadata intogenerateMetadata/headAll of these
<meta>/<link>nodes now render inside the page body, so browsers and crawlers ignore them for SEO and social previews. Only the title returned fromgenerateMetadatamakes it to the real<head>, which means we’ve effectively dropped canonical URL, description, OG/Twitter data, etc. Please migrate this configuration into the Metadata API (e.g. extend the existinggenerateMetadata/getAppMetadatacall) so the tags are emitted from the document head again.(nextjs.org)app/education/tweetstorms/page.tsx (1)
363-371: Pinned tweet link: title/href mismatch.Title points to a different status than href; align them to avoid confusion.
Apply one of:
- title="https://twitter.com/punk6529/status/1445468399656595456" - href="https://twitter.com/punk6529/status/1448399827054833668" + title="https://twitter.com/punk6529/status/1448399827054833668" + href="https://twitter.com/punk6529/status/1448399827054833668"app/education/education-collaboration-form/page.tsx (1)
413-423: Text inputs prefilled with "true".defaultValue="true" will render the literal string “true” in Name/Twitter/Discord fields. Remove defaultValue or set to empty.
Apply:
- defaultValue="true" + defaultValue=""Or drop the prop entirely for uncontrolled inputs.
Also applies to: 436-445, 459-467
🧹 Nitpick comments (33)
__tests__/helpers/Helpers.test.ts (1)
74-78: Please switch the target attribute to_blank.Great call adding
rel="noopener noreferrer", but the expectation still assertstarget="blank". Without the underscore this creates a named browsing context, so subsequent external links will reuse the same window instead of spawning fresh tabs. Let’s update both the helper and this test to usetarget="_blank"alongside the new rel value.__tests__/components/common/TimePicker.test.tsx (7)
1-1: Adopt user-event and remove fireEvent importUse @testing-library/user-event for realistic interactions and cleaner async flow. Update imports accordingly.
Apply:
-import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event';Also confirm jest-dom is configured once in test setup so matchers like toBeDisabled work. Based on learnings.
6-18: Make the test async; use user-event and assert the last callTyping into number inputs fires multiple change events. Assert the latest call to avoid flakiness.
-it('labels hour and minute inputs for accessibility', () => { +it('labels hour and minute inputs for accessibility', async () => { const onChange = jest.fn(); render(<TimePicker hours={9} minutes={15} onTimeChange={onChange} />); const hoursInput = screen.getByLabelText('Hours'); const minutesInput = screen.getByLabelText('Minutes'); - - fireEvent.change(hoursInput, { target: { value: '10' } }); - expect(onChange).toHaveBeenCalledWith(10, 15); - - fireEvent.change(minutesInput, { target: { value: '45' } }); - expect(onChange).toHaveBeenCalledWith(9, 45); + const user = userEvent.setup(); + await user.clear(hoursInput); + await user.type(hoursInput, '10'); + expect(onChange).toHaveBeenLastCalledWith(10, 15); + + await user.clear(minutesInput); + await user.type(minutesInput, '45'); + expect(onChange).toHaveBeenLastCalledWith(9, 45); });Based on learnings.
20-27: Use user-event for toggle click; make test asyncImproves realism and consistency with other tests.
-it('toggles am/pm respecting minTime', () => { +it('toggles am/pm respecting minTime', async () => { const onChange = jest.fn(); render( <TimePicker hours={9} minutes={0} onTimeChange={onChange} minTime={{ hours: 8, minutes: 0 }} /> ); - fireEvent.click(screen.getByRole('button', { name: 'Toggle AM/PM' })); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /toggle am\/pm/i })); expect(onChange).toHaveBeenCalledWith(21, 0); });Based on learnings.
38-46: Leverage jest-dom’s accessible description matcherThis directly tests the a11y contract and is robust to multiple ids in aria-describedby.
-const describedBy = hoursInput.getAttribute('aria-describedby'); -expect(describedBy).toBeTruthy(); - -const description = document.getElementById(describedBy ?? ''); -expect(description).not.toBeNull(); -expect(description).toHaveTextContent('Earliest selectable time is 9:30 AM.'); - -expect(minutesInput.getAttribute('aria-describedby')).toBe(describedBy); +expect(hoursInput).toHaveAccessibleDescription('Earliest selectable time is 9:30 AM.'); +expect(minutesInput).toHaveAccessibleDescription('Earliest selectable time is 9:30 AM.');Based on learnings.
47-51: Prefer role-based button queries; switch to user-event; remove castRole queries are more resilient; user-event keeps interaction semantics.
-const early = screen.getByText('9 AM') as HTMLButtonElement; -const later = screen.getByText('12 PM'); +const early = screen.getByRole('button', { name: '9 AM' }); +const later = screen.getByRole('button', { name: '12 PM' }); expect(early).toBeDisabled(); -fireEvent.click(later); +const user = userEvent.setup(); +await user.click(later); expect(onChange).toHaveBeenCalledWith(12, 0);Also mark this test async if not already.
10-11: Optional: query numeric inputs by roleYou can target number inputs via role for extra clarity:
screen.getByRole('spinbutton', { name: /hours/i }); screen.getByRole('spinbutton', { name: /minutes/i });Also applies to: 35-36
1-53: Remove redundant jest-dom imports jest-dom is already initialized viasetupFilesAfterEnvinjest.setup.js; please delete all individualimport '@testing-library/jest-dom'lines from your test files.app/museum/6529-fund-szn1/cryptoad-punks/page.tsx (4)
4-29: Move head/meta/link tags to Next metadata/headRendering meta/link/style tags inside the page body can cause invalid markup and hydration/SEO issues. Use generateMetadata and the root/layout head for these tags; remove body-inline head tags.
Also applies to: 30-43
89-93: Avoid dangerouslySetInnerHTML for CSS blocksInline CSS via dangerouslySetInnerHTML is unnecessary and trips security linters. Extract to CSS (or Tailwind), import at module scope, or embed as <style>{cssString}</style> if absolutely needed.
Also applies to: 105-107, 127-130, 135-138, 143-146, 241-246
405-416: Conflicting image priorities: loading="lazy" + fetchPriority="high"These conflict. Keep lazy and drop fetchPriority, or make the image eager if it’s the LCP.
Apply this diff:
- fetchPriority="high"
563-565: Add explicit type to generateMetadata for consistencyOther pages type this as Promise. Do the same here.
Apply this diff:
+import { Metadata } from "next"; -export async function generateMetadata() { +export async function generateMetadata(): Promise<Metadata> { return getAppMetadata({ title: "CRYPTOAD PUNKS - 6529.io" }); }app/author/nft6529/page.tsx (2)
7-58: Move head elements to metadata; keep body markup cleanMeta/link/title tags should be set via generateMetadata (or layout metadata), not rendered inside the page body. Convert robots, canonical, OG, Twitter, icons, preloads, and font/CSS includes to Next metadata or layout. This reduces duplicate tags and avoids hydration issues.
Also applies to: 66-160, 167-196
2-2: Use a type-only import for MetadataChange to a type import to avoid bundling types at runtime.
-import { Metadata } from "next"; +import type { Metadata } from "next";Also applies to: 338-340
app/museum/6529-fund-szn1/clonex/page.tsx (3)
4-77: Head/SEO resources should not live in the bodyMove canonical/OG/Twitter/meta/link/preload/icon/oEmbed tags into generateMetadata (and layout where appropriate). Keep the component tree for visible content only.
Also applies to: 181-206, 208-230, 239-256
12-17: Canonical and og:url should be absolutePrefer absolute URLs to avoid SEO ambiguity.
580-582: Type the metadata exportAnnotate return type for consistency with other pages.
-export async function generateMetadata() { +export async function generateMetadata(): Promise<import('next').Metadata> {app/category/blog/page.tsx (1)
7-37: Head/meta/link content belongs in metadata/layoutRelocate robots/canonical/OG/Twitter/feeds/icons/preloads to generateMetadata or layout.
Also applies to: 59-169, 170-206
app/museum/6529-fam/page.tsx (2)
4-77: Relocate head/meta/link resources to metadata/layoutMove robots/canonical/OG/Twitter/feeds/icons/preloads to generateMetadata or layout.
Also applies to: 115-180
330-332: Type the metadata exportAdd explicit return type for consistency.
-export async function generateMetadata() { +export async function generateMetadata(): Promise<import('next').Metadata> {app/museum/6529-fam-2021/page.tsx (2)
4-76: Move head/meta/link resources to metadata/layoutRelocate canonical/OG/Twitter/description/feeds/icons/preloads to generateMetadata and layout.
Also applies to: 91-186, 186-236
441-443: Type the metadata exportAdd explicit return type.
-export async function generateMetadata() { +export async function generateMetadata(): Promise<import('next').Metadata> {app/museum/6529-fund-szn1/an-incomparable-love/page.tsx (4)
10-10: TSX inline comments violate project guideline.Remove JSX comments in .tsx files. As per coding guidelines.
- {/* This site is optimized with the Yoast SEO plugin v23.9 - https://yoast.com/wordpress/plugins/seo/ */} - {/* fusion-row */} - {/* #main */} - {/* wrapper */} - {/* #boxed-wrapper */} + {/* removed per guideline */}Also applies to: 25-25, 528-535
107-120: Drop external WP CSS/font preloads; use Tailwind and next/font.Avoid pulling large WP stylesheets and raw font preloads from CDNs. Rebuild layout with Tailwind utilities and load fonts via next/font for perf and CSP safety. As per coding guidelines.
Also applies to: 145-186, 214-236
539-541: Avoid dead “#” anchors; use buttons or valid fragment targets.href="#" with no target harms a11y and can cause scroll jumps. Use a button with onClick scroll-to-top (client component) or link to a real fragment id.
- <a href="#" id="toTop" className="fusion-top-top-link"> + <a href="#top" id="toTop" className="fusion-top-top-link">Or convert to a in a client component.
Also applies to: 563-567
12-13: Canonical should be absolute (handled via metadata).Ensure the canonical URL is absolute when moved to generateMetadata (e.g., https://6529.io/...). As per coding guidelines.
app/capital/company-portfolio/page.tsx (2)
7-29: Move SEO/meta out of JSX into generateMetadata only.Avoid duplicating head tags inside the component; keep metadata in generateMetadata to prevent hydration/SEO duplication. Also prefer absolute canonical URLs.
As per coding guidelines.
389-401: Avoid mixing loading="lazy" with fetchPriority="high".These conflict; drop fetchPriority or set loading="eager" if this image is critical-path.
app/email-signatures/page.tsx (1)
7-33: Consolidate head tags into generateMetadata.Remove runtime /<title> from JSX to rely solely on generateMetadata to avoid duplication.
As per coding guidelines.
app/capital/fund/page.tsx (1)
7-29: Deduplicate metadata: keep in generateMetadata, remove from JSX.Prevents head duplication and keeps SEO config centralized.
As per coding guidelines.
app/education/tweetstorms/page.tsx (2)
392-922: Optional: trim tracking params from Twitter URLs.Consider removing query strings like ?s=20&t=… for cleaner, canonical links.
7-29: Centralize metadata via generateMetadata only.Remove inline head/meta tags from JSX to avoid duplication.
As per coding guidelines.
app/education/education-collaboration-form/page.tsx (1)
15-23: Avoid duplicating OG title/meta in JSX; keep metadata in generateMetadata.This page already exports generateMetadata—remove the JSX head/OG duplicates.
As per coding guidelines.
Also applies to: 49-55
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
__tests__/moreStaticPages.test.tsx (5)
35-37: Preserve other next/navigation exports when mockingOverriding the whole module can break pages that import additional exports. Spread the actual module and stub only useRouter.
-jest.mock("next/navigation", () => ({ - useRouter: jest.fn(), -})); +jest.mock("next/navigation", () => { + const actual = jest.requireActual("next/navigation"); + return { ...actual, useRouter: jest.fn() }; +});
69-74: Deduplicate router mocking and make the mock reusable across testsCreate a shared routerMock in the suite setup and return it from useRouter in beforeEach. Then drop the per-test re-mock.
+let routerMock: { back: jest.Mock; push: jest.Mock }; beforeEach(() => { - (useRouter as jest.Mock).mockReturnValue({ - back: jest.fn(), - push: jest.fn(), - }); + routerMock = { back: jest.fn(), push: jest.fn() }; + (useRouter as jest.Mock).mockReturnValue(routerMock); });
79-83: Use the shared routerMock from beforeEachAfter centralizing the mock, remove the local redefinition here.
- const routerMock = { back: jest.fn(), push: jest.fn() }; - (useRouter as jest.Mock).mockReturnValue(routerMock); - const user = userEvent.setup(); + const user = userEvent.setup();
39-41: Add readonly to props per TS guidelinesApply readonly to children in local/test providers.
-const TestProvider: React.FC<{ children: React.ReactNode }> = ({ +const TestProvider: React.FC<{ readonly children: React.ReactNode }> = ({ children, }) => {- TitleProvider: ({ children }: { children: React.ReactNode }) => children, + TitleProvider: ({ children }: { readonly children: React.ReactNode }) => children,Also applies to: 65-66
90-97: Guideline: avoid inline comments in .ts/.tsxProject guideline forbids comments in ts/tsx files. Consider removing the inline “Check for …” comments to comply.
Also applies to: 104-107
app/blog/disney-deekay-their-secret-to-animation/page.tsx (2)
1119-1125: Consider removing or fully hiding the “Page load link” button.It’s non-focusable but still operable/visible if not styled off-screen. If unused, remove it; otherwise add
hiddento ensure it doesn’t surface.Option A (remove):
- <button - type="button" - className="fusion-one-page-text-link fusion-page-load-link" - tabIndex={-1} - aria-hidden="true"> - Page load link - </button>Option B (keep but hide):
- <button + <button type="button" - className="fusion-one-page-text-link fusion-page-load-link" + className="fusion-one-page-text-link fusion-page-load-link" tabIndex={-1} - aria-hidden="true"> + aria-hidden="true" + hidden> Page load link </button>As per coding guidelines (TSX + TailwindCSS), if retained visibly later, prefer Tailwind utilities over theme classes.
1157-1159: Populate metadata here and drop in-body head tags to reduce duplication.Move page description, OG image, Twitter card, and canonical into
generateMetadata; then remove redundant<meta>/<link>tags from the body. Aligns with Next.js App Router patterns and project guidelines.Apply this diff:
export async function generateMetadata(): Promise<Metadata> { - return getAppMetadata({ - title: "Disney and DeeKay: Their Secret to Animation - 6529.io", - }); + const base = getAppMetadata({ + title: "Disney and DeeKay: Their Secret to Animation - 6529.io", + description: + "Some animators possess the talent to move us, rattle us, charm, inspire, delight and amaze in ways that make their art form endure. From Disney to DeeKay, these are the animators who are gifted with the secret recipe for animation that both reflects and affects our humanity.", + ogImage: + "https://dnclu2fna0b2b.cloudfront.net/wp-content/uploads/2023/02/deekay.jpeg", + twitterCard: "summary_large_image", + }); + return { + ...base, + alternates: { + canonical: + "https://6529.io/blog/disney-deekay-their-secret-to-animation/", + }, + }; }Then remove the duplicate meta/link tags rendered inside the component body (e.g., description, OG image, twitter:card, canonical), using
generateMetadatainstead. As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/moreStaticPages.test.tsx(4 hunks)app/blog/disney-deekay-their-secret-to-animation/page.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsUse TypeScript for implementation code
Files:
app/blog/disney-deekay-their-secret-to-animation/page.tsx__tests__/moreStaticPages.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks
Files:
app/blog/disney-deekay-their-secret-to-animation/page.tsx__tests__/moreStaticPages.test.tsx
{app,pages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use NextJS features that match the current version
Files:
app/blog/disney-deekay-their-secret-to-animation/page.tsx
app/**/page.tsx
📄 CodeRabbit inference engine (AGENTS.md)
All new pages must be created inside the app/ directory
Files:
app/blog/disney-deekay-their-secret-to-animation/page.tsx
app/**/{page,layout}.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Routes in app/ should export a generateMetadata function using getAppMetadata
Files:
app/blog/disney-deekay-their-secret-to-animation/page.tsx
**/{__tests__/**/*.{ts,tsx},*.test.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/{__tests__/**/*.{ts,tsx},*.test.tsx}: Place tests in tests directories or alongside components as ComponentName.test.tsx
Mock external dependencies and APIs in tests
Files:
__tests__/moreStaticPages.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/moreStaticPages.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-28T12:33:30.941Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:30.941Z
Learning: Applies to __tests__/components/**/*.{ts,tsx,js,jsx} : Use `testing-library/react` and `testing-library/user-event` for React component tests
Applied to files:
__tests__/moreStaticPages.test.tsx
🧬 Code graph analysis (1)
app/blog/disney-deekay-their-secret-to-animation/page.tsx (1)
components/providers/metadata.ts (1)
getAppMetadata(41-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
__tests__/moreStaticPages.test.tsx (2)
9-10: LGTM: user-event setup and Next router import are correctUsing userEvent with async tests and awaiting interactions is aligned with best practices. Based on learnings.
98-103: LGTM: back-button behavior is asserted via user interactionsThe click is awaited and the expectation checks the correct router method call. Based on learnings.
app/blog/disney-deekay-their-secret-to-animation/page.tsx (2)
438-441: Security hardening on external links looks good.Adding rel="noopener noreferrer" to target="_blank" links prevents window.opener leaks and suppresses Referrer. LGTM.
Also applies to: 716-719, 948-952, 956-962, 1003-1007, 1036-1041, 1132-1137
1146-1150: To‑Top control: a11y wiring is correct.Button has a programmatic label via the on-page SR text. Good.
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|


Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests