Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds feed layout (list/grid) support with a provider and UI, introduces a responsive Masonry renderer and caching, a StickyFilterBar, enforces list layout on several feeds, refactors filter/sidebar responsiveness, adjusts modal/popover scroll locking, increases banner z-index, and adds i18n keys ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Root as RootLayout
participant FLP as FeedLayoutProvider
participant Page as FeedPage
participant Feed as PaginatedPostsFeed/AwaitedPostsFeed
participant LayoutView as FeedLayoutView
participant Masonry as Masonry
participant Card as FeedItemCard
User->>Root: request feed page
Root->>Root: read FEED_LAYOUT_COOKIE or default
Root->>FLP: initialize with initialLayout
FLP->>Page: provide layout context
Page->>Feed: render feed (may pass forceLayout)
Feed->>Feed: resolve layout = forceLayout ?? contextLayout
Feed->>LayoutView: pass items & layout
LayoutView->>Masonry: configure columns/gap & balanced mode
Masonry->>Card: render each item
Card->>User: display post/project
User->>FLP: click LayoutSwitcher -> setLayout("grid")
FLP->>Feed: context updated -> re-render
sequenceDiagram
participant User
participant QPage as QuestionsPage
participant SFB as StickyFilterBar
participant IO as IntersectionObserver
participant Filters as FeedFilters
participant Feed as AwaitedPostsFeed
User->>QPage: open Questions page
QPage->>SFB: render StickyFilterBar(FeedFilters)
SFB->>IO: attach observer to sentinel
User->>QPage: scroll
IO->>SFB: update isStuck state
SFB->>SFB: toggle glass styling/top offset
SFB->>Filters: render filters (respect forceLayout)
Filters->>Feed: apply filters & render feed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
cemreinanc
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
front_end/src/app/(main)/components/content_translated_banner/index.tsx (1)
35-53:⚠️ Potential issue | 🟡 MinorLocalize the attribution label.
This banner block still hardcodes
translated by, so every locale gets English copy even though the rest of the component already usesuseTranslations(). Please move that label into the message catalog as well.Based on learnings: Do not hardcode English strings in TSX components; prefer
useTranslations()and i18n strings for UI text across components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/components/content_translated_banner/index.tsx around lines 35 - 53, The banner currently hardcodes the English label "translated by"; replace it with a localized string from the same translation hook used earlier (e.g., call t("translatedBy") or similar) in the component (index.tsx) and update the message catalog keys accordingly; ensure the new key exists in your locale files and use the same useTranslations() instance already imported/used in this component so the span renders t("translatedBy") instead of the hardcoded text.front_end/src/components/base_modal.tsx (1)
34-43:⚠️ Potential issue | 🟠 MajorGlobal scroll lock is still race-prone across overlays.
Line 36 and Line 42 mutate
document.body.style.overflowwithout coordination. Withfront_end/src/components/popover_filter/index.tsx(Line 33-40) doing the same, closing one overlay can restore scrolling while another overlay is still open.🔧 Proposed fix (reference-counted body scroll lock)
+const BODY_SCROLL_LOCK_COUNT_KEY = "overlayScrollLockCount"; + +const lockBodyScroll = () => { + const current = Number(document.body.dataset[BODY_SCROLL_LOCK_COUNT_KEY] ?? "0"); + const next = current + 1; + document.body.dataset[BODY_SCROLL_LOCK_COUNT_KEY] = String(next); + document.body.style.overflow = "clip"; +}; + +const unlockBodyScroll = () => { + const current = Number(document.body.dataset[BODY_SCROLL_LOCK_COUNT_KEY] ?? "0"); + const next = Math.max(0, current - 1); + if (next === 0) { + delete document.body.dataset[BODY_SCROLL_LOCK_COUNT_KEY]; + document.body.style.overflow = "unset"; + } else { + document.body.dataset[BODY_SCROLL_LOCK_COUNT_KEY] = String(next); + } +}; + useEffect(() => { - if (isOpen && isImmersive) { - document.body.style.overflow = "clip"; - } else { - document.body.style.overflow = "unset"; - } - - return () => { - document.body.style.overflow = "unset"; - }; + if (!(isOpen && isImmersive)) return; + lockBodyScroll(); + return () => unlockBodyScroll(); }, [isOpen, isImmersive]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/base_modal.tsx` around lines 34 - 43, The effect in base_modal.tsx that directly sets document.body.style.overflow is race-prone with other overlays (e.g., popover_filter) — replace the direct mutations with a shared reference-counted scroll-lock API (either a small module exposing acquireBodyScrollLock/releaseBodyScrollLock or a hook like useBodyScrollLock) and call that from the useEffect in base_modal (and update popover_filter to use the same API); ensure acquire increments a counter and sets overflow="clip" when counter goes from 0→1, and release decrements and only restores overflow when counter reaches 0, and call release in the effect cleanup.front_end/src/components/ui/listbox.tsx (1)
159-165:⚠️ Potential issue | 🟠 Major
menuPositionstill does not affect portal placement.
FloatingMenunow receives the prop, but the fixed-position style still always anchors toleft: rect.left. In portal mode, a menu wider than its button will therefore expand from the left edge even when the caller wants right alignment, which can push options off-screen near the viewport edge.Proposed fix
style={ !renderInPortal ? undefined : { position: "fixed", - left: rect?.left, + left: menuPosition === "left" ? rect?.left : undefined, + right: + menuPosition === "right" && rect + ? window.innerWidth - rect.right + : undefined, top: !flipUp ? rect?.bottom : undefined, bottom: flipUp ? window.innerHeight - (rect?.top ?? 0) : undefined,Also applies to: 191-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/listbox.tsx` around lines 159 - 165, FloatingMenu receives the menuPosition prop but portal placement still anchors using rect.left; update the portal positioning logic inside FloatingMenu (the branch that uses renderInPortal) to honor menuPosition ('left'|'right'|'center' etc.) by computing the horizontal offset from rect (e.g., use rect.right - menuWidth for 'right', rect.left for 'left', or center accordingly) or set the appropriate CSS right/left based on viewport bounds before applying fixed positioning; ensure the same change is applied to the other portal code path that mirrors this logic (the second FloatingMenu usage handling optionsClassName/menuMinWidthMatchesButton) so portal-mode menus align as requested.
🧹 Nitpick comments (2)
front_end/src/components/random_button.tsx (1)
38-38: Hardcodedaria-labelshould use i18n.The string
"Random Question"is hardcoded in English. Per the project's i18n conventions, useuseTranslations()instead to ensure this label is localized.♻️ Suggested fix
+import { useTranslations } from "next-intl"; + const RandomButton: FC<ComponentProps<typeof Button>> = ({ className, ...props }) => { + const t = useTranslations(); const [isLoading, setIsLoading] = useState(false); const router = useRouter(); ... return ( <Button onClick={handleRandomClick} disabled={isLoading} - aria-label="Random Question" + aria-label={t("randomQuestion")} size="md"Then add the
"randomQuestion"key to your translation files.Based on learnings: "Do not hardcode English strings in TSX components... Prefer using useTranslations() and i18n strings for UI text across components."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/random_button.tsx` at line 38, The aria-label on the RandomButton component is hardcoded as "Random Question"; replace it by calling useTranslations() in front_end/src/components/random_button.tsx (e.g., const t = useTranslations(...)) and use t("randomQuestion") for the aria-label and any visible text, and then add a "randomQuestion" key to your translation JSONs so the label is localized across locales.front_end/src/app/globals.css (1)
14-16: Scope this override narrower than any inlineoverflowon<html>.
html[style*="overflow"]matches every inline overflow mutation on the document root, so any legitimatedocument.documentElement.style.overflow = ...usage will now be silently undone by the base layer. Tightening this to the specific scroll-lock condition avoids another class of overlay regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/globals.css` around lines 14 - 16, The global CSS rule is too broad: replace the selector html[style*="overflow"] with a narrower check that only matches the scroll-lock case (e.g. html[style*="overflow: hidden"] or the exact inline style string your overlay library sets) so you only override the document root when the page is actually scroll-locked; update the combined selector (html:has(`#headlessui-portal-root`), html[style*="overflow"]) to use html:has(`#headlessui-portal-root`) and html[style*="overflow: hidden"] (or the exact inline style used) so legitimate programmatic mutations to document.documentElement.style.overflow are not globally undone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front_end/src/app/`(main)/questions/components/feed_filters/main.tsx:
- Around line 126-128: The memoized value mainSortNewsVisible is missing
PUBLIC_MINIMAL_UI in its useMemo dependency array, causing stale values when
PUBLIC_MINIMAL_UI changes; update the useMemo that computes mainSortNewsVisible
to include PUBLIC_MINIMAL_UI alongside t, isLargeScreen, and isMediumScreen so
the memo recomputes when PUBLIC_MINIMAL_UI updates.
In `@front_end/src/app/layout.tsx`:
- Around line 98-99: The current assignment to feedLayout casts the cookie value
directly to FeedLayout which trusts client-controlled FEED_LAYOUT_COOKIE; change
it to validate the cookie value returned by
cookieStore.get(FEED_LAYOUT_COOKIE)?.value and only accept it if it strictly
equals "list" or "grid", otherwise use FEED_LAYOUT_DEFAULT so FeedLayoutProvider
receives a safe initialLayout; update the logic around feedLayout (and any code
relying on that value) to remove the unchecked cast and perform this whitelist
check before passing the value into FeedLayoutProvider.
In `@front_end/src/components/search_input.tsx`:
- Around line 59-60: isExpanded is only initialized from value and can get out
of sync when the parent changes value; add a useEffect that watches value (and
collapsible) and calls setIsExpanded(!!value) so the input expands whenever a
non-empty value is provided externally. Locate the isExpanded state and updater
(isExpanded, setIsExpanded) and add the effect to synchronize them, ensuring the
effect no-ops when collapsible is false to preserve current behavior; also apply
the same synchronization in the other related block that manages expansion (the
code referenced around the 86-108 region).
- Around line 88-91: The collapse icon button currently uses
aria-label={placeholder} which can be empty; change the component API to require
a localized label prop (e.g., collapseButtonLabel or ariaLabel) when rendering
in collapsible mode and use that prop for the button aria-label instead of
placeholder; update the SearchInput component (and any caller sites) to pass an
i18n-backed string, validate that collapseButtonLabel is present when
collapsible/expand mode is enabled (throw or assert), and remove any hardcoded
English fallback so the button always has an accessible name.
- Around line 62-71: The expand() callback currently only handles object refs
and thus fails to focus when a callback ref is passed via prop inputRef; update
expand (and the ref usage) to handle both object refs and callback refs by: if
inputRef is a function, call it with null/element as needed or store the element
temporarily and call inputRef(element) so you can focus that element, otherwise
use inputRef.current or internalRef.current; ensure the ref passed on the input
uses the same logic so internalRef is set when inputRef is a callback. Also add
a fallback aria-label for the collapsed icon button (use placeholder || "Open
search" or a similar descriptive default) so the button is accessible when
placeholder is undefined. Reference expand, inputRef, internalRef, and the
collapsed icon button/ref prop when making changes.
In `@front_end/src/components/ui/layout_switcher.tsx`:
- Around line 25-50: Replace the hardcoded aria-label strings in the
LayoutSwitcher component with localized strings from useTranslations(): import
and call useTranslations() at the top of the component (e.g., const t =
useTranslations()), then change aria-label="List layout" to
aria-label={t('layout.list')} and aria-label="Grid layout" to
aria-label={t('layout.grid')} (or use the appropriate i18n keys your project
uses); keep the existing onChange and value logic intact so the buttons still
call onChange("list") / onChange("grid").
In `@front_end/src/components/ui/masonry.tsx`:
- Around line 74-84: The Masonry component currently uses indexes/object
identity for keys and in its internal Map bookkeeping which breaks stability
when items are recreated or redistributed; add a getKey: (item: T, idx: number)
=> string|number to MasonryProps and use getKey(item, idx) everywhere instead of
idx or item identity—use that value for the React key on rendered items (in
render wrapper) and as the Map key in placement tracking and any maps that track
item -> column/position (update any usages inside functions that compute
balanced placement and column assignment); ensure getKey is passed through to
the places where columns are built and where items are iterated so the same
stable key is used both for React keys and all internal maps.
- Around line 321-329: The createDataColumns function incorrectly treats falsy
values as missing by using a truthy check on item; update the condition in the
inner loop to explicitly check for undefined (e.g., item !== undefined) so
values like 0, false, and "" are preserved; locate the check that references
item within createDataColumns (and the variables result, data, columns) and
replace the truthy guard with an explicit undefined comparison.
- Around line 21-31: The initial state in useMediaValues is set to columns: 0
which causes Masonry to return null on first render; change the useState
initializer in useMediaValues to derive its default from the base layout values
(e.g. setValues initial object to { columns: columns?.[0] ?? 0, gap: gap?.[0] ??
0 }) so the first render uses the base columns/gap instead of 0, and apply the
same change to the other useState occurrence for this hook (the second
initializer around the 95-99 area) so both initial states mirror the provided
columns[0]/gap[0].
---
Outside diff comments:
In `@front_end/src/app/`(main)/components/content_translated_banner/index.tsx:
- Around line 35-53: The banner currently hardcodes the English label
"translated by"; replace it with a localized string from the same translation
hook used earlier (e.g., call t("translatedBy") or similar) in the component
(index.tsx) and update the message catalog keys accordingly; ensure the new key
exists in your locale files and use the same useTranslations() instance already
imported/used in this component so the span renders t("translatedBy") instead of
the hardcoded text.
In `@front_end/src/components/base_modal.tsx`:
- Around line 34-43: The effect in base_modal.tsx that directly sets
document.body.style.overflow is race-prone with other overlays (e.g.,
popover_filter) — replace the direct mutations with a shared reference-counted
scroll-lock API (either a small module exposing
acquireBodyScrollLock/releaseBodyScrollLock or a hook like useBodyScrollLock)
and call that from the useEffect in base_modal (and update popover_filter to use
the same API); ensure acquire increments a counter and sets overflow="clip" when
counter goes from 0→1, and release decrements and only restores overflow when
counter reaches 0, and call release in the effect cleanup.
In `@front_end/src/components/ui/listbox.tsx`:
- Around line 159-165: FloatingMenu receives the menuPosition prop but portal
placement still anchors using rect.left; update the portal positioning logic
inside FloatingMenu (the branch that uses renderInPortal) to honor menuPosition
('left'|'right'|'center' etc.) by computing the horizontal offset from rect
(e.g., use rect.right - menuWidth for 'right', rect.left for 'left', or center
accordingly) or set the appropriate CSS right/left based on viewport bounds
before applying fixed positioning; ensure the same change is applied to the
other portal code path that mirrors this logic (the second FloatingMenu usage
handling optionsClassName/menuMinWidthMatchesButton) so portal-mode menus align
as requested.
---
Nitpick comments:
In `@front_end/src/app/globals.css`:
- Around line 14-16: The global CSS rule is too broad: replace the selector
html[style*="overflow"] with a narrower check that only matches the scroll-lock
case (e.g. html[style*="overflow: hidden"] or the exact inline style string your
overlay library sets) so you only override the document root when the page is
actually scroll-locked; update the combined selector
(html:has(`#headlessui-portal-root`), html[style*="overflow"]) to use
html:has(`#headlessui-portal-root`) and html[style*="overflow: hidden"] (or the
exact inline style used) so legitimate programmatic mutations to
document.documentElement.style.overflow are not globally undone.
In `@front_end/src/components/random_button.tsx`:
- Line 38: The aria-label on the RandomButton component is hardcoded as "Random
Question"; replace it by calling useTranslations() in
front_end/src/components/random_button.tsx (e.g., const t =
useTranslations(...)) and use t("randomQuestion") for the aria-label and any
visible text, and then add a "randomQuestion" key to your translation JSONs so
the label is localized across locales.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d5673d5-1c76-40de-93a3-a6755c8fc49e
📒 Files selected for processing (37)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/app/(main)/(tournaments)/tournament/components/tournament_feed.tsxfront_end/src/app/(main)/accounts/profile/[id]/questions/page.tsxfront_end/src/app/(main)/c/[slug]/page.tsxfront_end/src/app/(main)/c/[slug]/settings/page.tsxfront_end/src/app/(main)/components/content_translated_banner/index.tsxfront_end/src/app/(main)/components/global_search.tsxfront_end/src/app/(main)/news/page.tsxfront_end/src/app/(main)/questions/components/feed_filters/main.tsxfront_end/src/app/(main)/questions/components/feed_filters/my_predictions.tsxfront_end/src/app/(main)/questions/components/feed_filters/my_questions_and_posts.tsxfront_end/src/app/(main)/questions/components/sidebar.tsxfront_end/src/app/(main)/questions/components/sticky_filter_bar.tsxfront_end/src/app/(main)/questions/page.tsxfront_end/src/app/globals.cssfront_end/src/app/layout.tsxfront_end/src/components/base_modal.tsxfront_end/src/components/communities_feed/paginated_communities_feed.tsxfront_end/src/components/popover_filter/index.tsxfront_end/src/components/posts_feed/build_feed_items.tsfront_end/src/components/posts_feed/index.tsxfront_end/src/components/posts_feed/paginated_feed.tsxfront_end/src/components/posts_filters.tsxfront_end/src/components/random_button.tsxfront_end/src/components/search_input.tsxfront_end/src/components/tournament_filters.tsxfront_end/src/components/ui/layout_switcher.tsxfront_end/src/components/ui/listbox.tsxfront_end/src/components/ui/masonry.tsxfront_end/src/components/ui/tailwind-indicator.tsxfront_end/src/constants/posts_feed.tsfront_end/src/contexts/feed_layout_context.tsx
💤 Files with no reviewable changes (1)
- front_end/src/app/(main)/components/global_search.tsx
cemreinanc
changed the title
ncarazon
requested changes
Mar 30, 2026
Comment on lines
+151
to
+182
| if (columns !== prevColumnCount.current) { | ||
| columnAssignments.current.clear(); | ||
| measuredHeights.current.clear(); | ||
| prevColumnCount.current = columns; | ||
| } | ||
|
|
||
| // Stale cleanup: remove items no longer in the array | ||
| const itemSet = new Set(items); | ||
| for (const key of columnAssignments.current.keys()) { | ||
| if (!itemSet.has(key)) { | ||
| columnAssignments.current.delete(key); | ||
| measuredHeights.current.delete(key); | ||
| } | ||
| } | ||
| for (const key of measuredHeights.current.keys()) { | ||
| if (!itemSet.has(key)) { | ||
| measuredHeights.current.delete(key); | ||
| } | ||
| } | ||
|
|
||
| const placedItems: T[] = []; | ||
| const pendingItems: T[] = []; | ||
| for (const item of items) { | ||
| if (columnAssignments.current.has(item)) { | ||
| placedItems.push(item); | ||
| } else { | ||
| pendingItems.push(item); | ||
| } | ||
| } | ||
|
|
||
| // Build columns for placed items | ||
| const placedColumns: T[][] = Array.from({ length: columns }, () => []); |
Contributor
There was a problem hiding this comment.
BalancedMasonry performs side effects during render (clearing refs, iterating Maps). Anti-pattern for React strict mode and concurrent features. Move to useEffect.
Contributor
Author
There was a problem hiding this comment.
Those render-time mutations only actually do anything in two specific cases:
- The column count changed (breakpoint shift), it clears the maps so items get redistributed.
- Items were removed from the array, it deletes stale keys so the maps don't leak.
Outside of those two cases, every single one of those loops and conditionals is a no-op. The if conditional doesn't fire. The for loops iterate the maps and delete nothing because every key is still in itemSet. The code runs, does zero mutations, and falls through.
And even when they do fire, Strict Mode's double-render would just clear-and-rebuild identically both times; the result is the same and idempotent.
Moving this to useEffect would be strictly worse: you'd render one frame with stale data (wrong column count, dead items still assigned), paint that to the DOM, then fix the state or refs and re-render. It will cause a visible flicker for no reason. The whole point is that the refs need to be correct before the render output is computed, not after (like useEffect).
I couldn't find any other solution that fits these constraints and even if we mutate the ref contents still its usage seemed pretty careful. Any suggestions after considering these?
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
front_end/src/app/globals.css (1)
14-22:⚠️ Potential issue | 🟠 MajorScope these overflow overrides to HeadlessUI-managed state only.
Line 14 and Line 19 still match any inline root overflow styles, so unrelated modal/dialog flows can be overridden by
overflow: clip/unset !important, causing scroll-lock leaks.#!/bin/bash # Verify whether non-HeadlessUI code also mutates root overflow/padding-right, # which would be affected by the global selectors in globals.css. rg -n -C3 --type=ts --type=tsx --type=js --type=jsx \ 'document\.(documentElement|body)\.style\.(overflow|paddingRight)|setProperty\(\s*["'"'"'`](overflow|padding-right)' \ front_end/src # Optional: quick scan for custom modal/dialog implementations that may rely on their own scroll-lock behavior. rg -n -C2 --type=ts --type=tsx --type=js --type=jsx \ '\b(Modal|Dialog|Drawer|Popover)\b' \ front_end/src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/globals.css` around lines 14 - 22, The global overflow overrides currently target any inline root overflow via selectors html[style*="overflow: hidden"], html[style*="overflow:hidden"], body[style*="overflow: auto"], body[style*="overflow:auto"], which can break non-HeadlessUI flows; restrict those selectors to only apply when HeadlessUI-managed state is active by adding a dedicated scoping attribute/class (e.g. html[data-headlessui-dialog-open], html[data-headlessui-modal-open] or .headlessui-modal-open) so the rules only run for HeadlessUI modals/dialogs; update the CSS selectors in globals.css to include that attribute/class and ensure the HeadlessUI wrapper that controls opening/closing sets and removes that attribute/class when Dialog/Modal opens/closes.
🧹 Nitpick comments (2)
front_end/src/components/ui/masonry.tsx (2)
152-171: Side effects during render are anti-pattern for React concurrent mode.Lines 152-171 mutate
columnAssignments.currentandmeasuredHeights.currentdirectly during render. In React's concurrent mode and strict mode, render functions may be called multiple times before committing, which could lead to inconsistent state if these mutations are re-executed.Consider moving these cleanup operations to a
useEffectoruseLayoutEffectthat runs whencolumnsoritemschange.♻️ Suggested approach
+ useEffect(() => { + // Breakpoint change: clear everything for full redistribution + columnAssignments.current.clear(); + measuredHeights.current.clear(); + }, [columns]); + + useEffect(() => { + // Stale cleanup: remove items no longer in the array + const itemSet = new Set(items); + for (const key of columnAssignments.current.keys()) { + if (!itemSet.has(key)) { + columnAssignments.current.delete(key); + measuredHeights.current.delete(key); + } + } + for (const key of measuredHeights.current.keys()) { + if (!itemSet.has(key)) { + measuredHeights.current.delete(key); + } + } + }, [items]); - // Breakpoint change: clear everything for full redistribution - if (columns !== prevColumnCount.current) { - columnAssignments.current.clear(); - measuredHeights.current.clear(); - prevColumnCount.current = columns; - } - - // Stale cleanup: remove items no longer in the array - const itemSet = new Set(items); - for (const key of columnAssignments.current.keys()) { - ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/masonry.tsx` around lines 152 - 171, The render is mutating refs (columnAssignments.current, measuredHeights.current, prevColumnCount.current) which is unsafe in concurrent renders; move the breakpoint-reset and stale-item cleanup into a hook (useEffect or useLayoutEffect) that runs when columns or items change; specifically, remove the in-render blocks that clear or delete from columnAssignments.current and measuredHeights.current and implement a useLayoutEffect/useEffect whose dependency array includes [columns, items] that performs: if columns !== prevColumnCount.current then clear both refs and set prevColumnCount.current = columns, and also iterate current keys to delete any entries not present in the new items Set.
236-237: Consider documenting the suppressed lint rule.The
eslint-disablecomment suppresses exhaustive-deps but doesn't explain the rationale.pendingItemsis derived fromitemsduring render, so includingpendingItems.lengthrather thanpendingItemsis intentional to avoid reference instability. A brief comment would help future maintainers understand this choice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/components/ui/masonry.tsx` around lines 236 - 237, Add a brief explanatory comment above the eslint-disable-next-line react-hooks/exhaustive-deps that states why the rule is suppressed: explain that pendingItems is derived from items during render and its object identity can change each render, so the effect intentionally depends on items, columns, and pendingItems.length (not pendingItems) to avoid unnecessary re-runs; keep the eslint-disable-next-line line but document the rationale referencing the useEffect dependency array and the variables items, columns, and pendingItems.length so future maintainers understand the choice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@front_end/src/app/globals.css`:
- Around line 14-22: The global overflow overrides currently target any inline
root overflow via selectors html[style*="overflow: hidden"],
html[style*="overflow:hidden"], body[style*="overflow: auto"],
body[style*="overflow:auto"], which can break non-HeadlessUI flows; restrict
those selectors to only apply when HeadlessUI-managed state is active by adding
a dedicated scoping attribute/class (e.g. html[data-headlessui-dialog-open],
html[data-headlessui-modal-open] or .headlessui-modal-open) so the rules only
run for HeadlessUI modals/dialogs; update the CSS selectors in globals.css to
include that attribute/class and ensure the HeadlessUI wrapper that controls
opening/closing sets and removes that attribute/class when Dialog/Modal
opens/closes.
---
Nitpick comments:
In `@front_end/src/components/ui/masonry.tsx`:
- Around line 152-171: The render is mutating refs (columnAssignments.current,
measuredHeights.current, prevColumnCount.current) which is unsafe in concurrent
renders; move the breakpoint-reset and stale-item cleanup into a hook (useEffect
or useLayoutEffect) that runs when columns or items change; specifically, remove
the in-render blocks that clear or delete from columnAssignments.current and
measuredHeights.current and implement a useLayoutEffect/useEffect whose
dependency array includes [columns, items] that performs: if columns !==
prevColumnCount.current then clear both refs and set prevColumnCount.current =
columns, and also iterate current keys to delete any entries not present in the
new items Set.
- Around line 236-237: Add a brief explanatory comment above the
eslint-disable-next-line react-hooks/exhaustive-deps that states why the rule is
suppressed: explain that pendingItems is derived from items during render and
its object identity can change each render, so the effect intentionally depends
on items, columns, and pendingItems.length (not pendingItems) to avoid
unnecessary re-runs; keep the eslint-disable-next-line line but document the
rationale referencing the useEffect dependency array and the variables items,
columns, and pendingItems.length so future maintainers understand the choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74a0cadf-221d-48ab-822f-9ba9d3d1beb5
📒 Files selected for processing (8)
front_end/src/app/(main)/components/global_search.tsxfront_end/src/app/globals.cssfront_end/src/app/layout.tsxfront_end/src/components/ui/masonry.tsxfront_end/src/components/visibility_observer.tsxfront_end/src/constants/posts_feed.tsfront_end/src/contexts/feed_layout_context.tsxfront_end/src/contexts/global_search_context.tsx
💤 Files with no reviewable changes (2)
- front_end/src/components/visibility_observer.tsx
- front_end/src/contexts/global_search_context.tsx
✅ Files skipped from review due to trivial changes (1)
- front_end/src/constants/posts_feed.ts
…dias is not available
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.
Notes:
Summary by CodeRabbit
New Features
Improvements
Localization