Added resizable sidebar with position toggle#16
Open
Kamkmgamer wants to merge 2 commits intoT3-Content:mainfrom
Open
Added resizable sidebar with position toggle#16Kamkmgamer wants to merge 2 commits intoT3-Content:mainfrom
Kamkmgamer wants to merge 2 commits intoT3-Content:mainfrom
Conversation
- Sidebar can be resized by dragging the edge (side: left edge, bottom: top edge) - Added toggle button to move sidebar between side and bottom positions - Bottom position displays standings in a horizontal grid layout - Side position maintains vertical list with configurable width (200-500px) - Bottom position height configurable (120-400px)
📝 WalkthroughWalkthroughAdds a resizable standings UI that can be toggled between a side and bottom position; implements drag-to-resize handling and a toggle control, with corresponding CSS updates for layout, positioning, and responsive behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Standings as Standings Component
participant App as App State
participant DOM as Browser DOM
rect rgba(100, 200, 100, 0.5)
Note over User,DOM: Toggle Position Flow
User->>Standings: Click toggle button
Standings->>App: call onTogglePosition()
App->>App: update sidebarPosition
App->>Standings: re-render with new position
Standings->>DOM: update layout class & styles
DOM->>User: display standings in new position
end
rect rgba(100, 150, 200, 0.5)
Note over User,DOM: Resize Flow
User->>DOM: mousedown on resize handle
Standings->>Standings: set isResizing = true
User->>DOM: mousemove
Standings->>Standings: compute new width/height
Standings->>App: call onResize(newWidth)
App->>App: update sidebarWidth
App->>Standings: re-render with new width
Standings->>DOM: apply inline size styles
DOM->>User: show resized standings
User->>DOM: mouseup
Standings->>Standings: set isResizing = false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Add a resizable standings panel with a position toggle between side and bottom in frontend.tsx and styles in frontend.cssIntroduce a draggable resize handle and position toggle in the 📍Where to StartStart with the Macroscope summarized cf47537. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend.tsx (4)
359-360:resizeRefis declared and attached but never read — dead code.
resizeRefis attached to the resize-handle<div>but is never accessed anywhere in the component. The mouse event handlers compute positions fromwindow.innerWidth/window.innerHeightonly and have no need for a reference to the handle element. It can be removed entirely.♻️ Proposed fix
- const resizeRef = useRef<HTMLDivElement>(null); const [isResizing, setIsResizing] = useState(false);<div - ref={resizeRef} className={`standings__resize-handle standings__resize-handle--${position}`} onMouseDown={() => setIsResizing(true)} />Also applies to: 393-395
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 359 - 360, The resizeRef created by useRef<HTMLDivElement>(null) and its attachment to the resize-handle div are dead — the component never reads it; remove the resizeRef declaration (useRef) and remove the ref={resizeRef} from the resize-handle JSX so the component only uses window measurements in the mouse handlers; ensure no other code references resizeRef (e.g., event handlers or cleanup) and delete any now-unused imports or variables related to it.
338-347:onResizeprop andsidebarWidthstate are both named for width but carry height in bottom mode.The
onResize: (width: number) => voidsignature, thesidebarWidthstate, and the inline style{ height:${width}px}all use width-oriented naming for a value that is height whenposition === "bottom". Consider a more neutral name (size/dimension) or split into two distinct state values to make the contract clear at call sites.♻️ Suggested rename (App-level state)
- const [sidebarWidth, setSidebarWidth] = useState(SIDEBAR_DEFAULT_WIDTH); + const [sidebarSize, setSidebarSize] = useState(SIDEBAR_DEFAULT_WIDTH);onResize: (size: number) => void;- style={position === "side" ? { width: `${width}px` } : { height: `${width}px` }} + style={position === "side" ? { width: `${size}px` } : { height: `${size}px` }}Also applies to: 388-391
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 338 - 347, The prop and state names treat a dimension as "width" even when it represents height in bottom mode—rename the ambiguous identifiers and update usages: change the prop onResize(width: number) to onResize(size: number) (or split into onResizeWidth/onResizeHeight) and rename sidebarWidth state to sidebarSize (or maintain sidebarWidth and sidebarHeight separately) and update all uses of sidebarWidth, the inline style { height: `${width}px` } and any call sites that pass or expect a width to use the new neutral name (e.g., onResize(size)) or the appropriate width/height-specific prop when position === "side" vs "bottom"; ensure type annotations, function signatures, and JSX style attributes match the new names (referencing onResize, sidebarWidth, position, and the inline style block).
330-332: Bottom resize bounds are magic numbers; side bounds use named constants.
SIDEBAR_MIN_WIDTH/SIDEBAR_MAX_WIDTHare defined as constants for the side position, but the bottom height constraints120and400are inlined at the call site. Add named constants for consistency and maintainability.♻️ Proposed fix
const SIDEBAR_MIN_WIDTH = 200; const SIDEBAR_MAX_WIDTH = 500; const SIDEBAR_DEFAULT_WIDTH = 280; +const SIDEBAR_MIN_HEIGHT = 120; +const SIDEBAR_MAX_HEIGHT = 400;- onResize(Math.min(400, Math.max(120, newHeight))); + onResize(Math.min(SIDEBAR_MAX_HEIGHT, Math.max(SIDEBAR_MIN_HEIGHT, newHeight)));Also applies to: 370-372
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 330 - 332, Define named constants BOTTOM_MIN_HEIGHT = 120 and BOTTOM_MAX_HEIGHT = 400 (similar to SIDEBAR_MIN_WIDTH/SIDEBAR_MAX_WIDTH) and replace the inline literals 120 and 400 at the bottom resize call sites with these constants; update both occurrences mentioned (around the existing SIDEBAR_* constants and the later call site at 370-372) so bottom bounds are consistent and maintainable using BOTTOM_MIN_HEIGHT and BOTTOM_MAX_HEIGHT.
473-476:setSidebarWidthintoggleSidebarPositionreads stale-safesidebarPosition, but a functional updater avoids the closure dependency entirely.The current implementation works correctly (both
setSidebarPositionandsetSidebarWidthare called synchronously before any re-render, sosidebarPositionreflects the pre-toggle value). However, nestingsetSidebarWidthinside thesetSidebarPositionfunctional updater removessidebarPositionfrom theuseCallbackdependency array and makes the pairing of the two state updates atomic and explicit:♻️ Optional refactor
const toggleSidebarPosition = useCallback(() => { - setSidebarPosition((p) => (p === "side" ? "bottom" : "side")); - setSidebarWidth(sidebarPosition === "side" ? 180 : SIDEBAR_DEFAULT_WIDTH); - }, [sidebarPosition]); + setSidebarPosition((p) => { + setSidebarWidth(p === "side" ? 180 : SIDEBAR_DEFAULT_WIDTH); + return p === "side" ? "bottom" : "side"; + }); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 473 - 476, The toggleSidebarPosition callback reads stale sidebarPosition when calling setSidebarWidth; change setSidebarPosition to use its functional updater and move the setSidebarWidth call inside that updater so it computes the new width from the previous position (use p param) — update the body of toggleSidebarPosition to call setSidebarPosition(p => { const next = p === "side" ? "bottom" : "side"; setSidebarWidth(next === "side" ? 180 : SIDEBAR_DEFAULT_WIDTH); return next; }) and remove sidebarPosition from the useCallback dependency array; reference: toggleSidebarPosition, setSidebarPosition, setSidebarWidth, sidebarPosition, SIDEBAR_DEFAULT_WIDTH.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend.css`:
- Around line 712-718: The .standings--bottom rule is capping the bottom panel
at 200px and overriding the inline style set by the sidebar in bottom mode (the
TSX sets style={{ height: `${width}px` }} with a JS-enforced 120–400px range),
so remove or disable the max-height: 200px declaration from .standings--bottom
so the inline height can take effect; ensure the CSS no longer imposes a
max-height on the .standings--bottom element so resizing up to 400px works as
intended.
In `@frontend.tsx`:
- Line 395: The onMouseDown handler currently calls setIsResizing(true) directly
which allows the browser to select text during dragging; change the arrow
handler to accept the event (e) and call e.preventDefault() before calling
setIsResizing(true) so the default text-selection behavior is suppressed (update
the onMouseDown arrow function where setIsResizing is used).
---
Nitpick comments:
In `@frontend.tsx`:
- Around line 359-360: The resizeRef created by useRef<HTMLDivElement>(null) and
its attachment to the resize-handle div are dead — the component never reads it;
remove the resizeRef declaration (useRef) and remove the ref={resizeRef} from
the resize-handle JSX so the component only uses window measurements in the
mouse handlers; ensure no other code references resizeRef (e.g., event handlers
or cleanup) and delete any now-unused imports or variables related to it.
- Around line 338-347: The prop and state names treat a dimension as "width"
even when it represents height in bottom mode—rename the ambiguous identifiers
and update usages: change the prop onResize(width: number) to onResize(size:
number) (or split into onResizeWidth/onResizeHeight) and rename sidebarWidth
state to sidebarSize (or maintain sidebarWidth and sidebarHeight separately) and
update all uses of sidebarWidth, the inline style { height: `${width}px` } and
any call sites that pass or expect a width to use the new neutral name (e.g.,
onResize(size)) or the appropriate width/height-specific prop when position ===
"side" vs "bottom"; ensure type annotations, function signatures, and JSX style
attributes match the new names (referencing onResize, sidebarWidth, position,
and the inline style block).
- Around line 330-332: Define named constants BOTTOM_MIN_HEIGHT = 120 and
BOTTOM_MAX_HEIGHT = 400 (similar to SIDEBAR_MIN_WIDTH/SIDEBAR_MAX_WIDTH) and
replace the inline literals 120 and 400 at the bottom resize call sites with
these constants; update both occurrences mentioned (around the existing
SIDEBAR_* constants and the later call site at 370-372) so bottom bounds are
consistent and maintainable using BOTTOM_MIN_HEIGHT and BOTTOM_MAX_HEIGHT.
- Around line 473-476: The toggleSidebarPosition callback reads stale
sidebarPosition when calling setSidebarWidth; change setSidebarPosition to use
its functional updater and move the setSidebarWidth call inside that updater so
it computes the new width from the previous position (use p param) — update the
body of toggleSidebarPosition to call setSidebarPosition(p => { const next = p
=== "side" ? "bottom" : "side"; setSidebarWidth(next === "side" ? 180 :
SIDEBAR_DEFAULT_WIDTH); return next; }) and remove sidebarPosition from the
useCallback dependency array; reference: toggleSidebarPosition,
setSidebarPosition, setSidebarWidth, sidebarPosition, SIDEBAR_DEFAULT_WIDTH.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
frontend.tsx (4)
359-359:resizeRefis assigned but never read — dead code.The ref is attached to the resize-handle
<div>but no code ever accessesresizeRef.current. If you intended to use it for bounding-rect calculations or similar, it's missing; otherwise remove it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` at line 359, The declared resizeRef (const resizeRef = useRef<HTMLDivElement>(null)) is never read; remove the dead code by deleting the resizeRef declaration and the ref={resizeRef} prop on the resize-handle <div> (or alternatively, if you actually need bounding-rect access, implement usage where you call resizeRef.current.getBoundingClientRect() in the relevant handler); update any imports if useRef becomes unused.
473-478: Side-effect inside a state updater —setSidebarWidthshould not be called withinsetSidebarPosition's updater.React requires updater functions to be pure. Calling another state setter inside one is a side-effect and will fire twice under
<StrictMode>. It works here only because the call is idempotent, but it violates the documented contract and is fragile.♻️ Proposed fix — derive both values, then set
const toggleSidebarPosition = useCallback(() => { - setSidebarPosition((p) => { - setSidebarWidth(p === "side" ? 180 : SIDEBAR_DEFAULT_WIDTH); - return p === "side" ? "bottom" : "side"; - }); + setSidebarPosition((p) => (p === "side" ? "bottom" : "side")); + setSidebarPosition((prev) => { + // Read the *just-set* value to derive the matching size + setSidebarWidth(prev === "bottom" ? SIDEBAR_DEFAULT_WIDTH : 180); + return prev; // keep position unchanged + }); }, []);Or, cleaner — combine both into a single state object:
- const [sidebarPosition, setSidebarPosition] = useState<"side" | "bottom">("side"); - const [sidebarWidth, setSidebarWidth] = useState(SIDEBAR_DEFAULT_WIDTH); + const [sidebar, setSidebar] = useState<{ position: "side" | "bottom"; size: number }>({ + position: "side", + size: SIDEBAR_DEFAULT_WIDTH, + }); const toggleSidebarPosition = useCallback(() => { - setSidebarPosition((p) => { - setSidebarWidth(p === "side" ? 180 : SIDEBAR_DEFAULT_WIDTH); - return p === "side" ? "bottom" : "side"; - }); + setSidebar((prev) => + prev.position === "side" + ? { position: "bottom", size: 180 } + : { position: "side", size: SIDEBAR_DEFAULT_WIDTH } + ); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 473 - 478, The updater passed to setSidebarPosition (inside toggleSidebarPosition) performs a side-effect by calling setSidebarWidth, which violates React's requirement that state updaters be pure; refactor toggleSidebarPosition so you compute the next position first (using current state or a functional read if needed) and then call setSidebarPosition and setSidebarWidth as separate calls outside any updater, or alternatively combine both into one state object (e.g., sidebarState) and update them together; reference toggleSidebarPosition, setSidebarPosition, setSidebarWidth and SIDEBAR_DEFAULT_WIDTH when locating the change.
370-371: Bottom-position min/max are magic numbers; extract constants like the side position.Side mode uses
SIDEBAR_MIN_WIDTH/SIDEBAR_MAX_WIDTH, but bottom mode hard-codes120and400. This also appears intoggleSidebarPosition(line 475 sets180). ExtractSIDEBAR_MIN_HEIGHT,SIDEBAR_MAX_HEIGHT, andSIDEBAR_DEFAULT_HEIGHTfor consistency and single-source-of-truth.♻️ Proposed constants
const SIDEBAR_MIN_WIDTH = 200; const SIDEBAR_MAX_WIDTH = 500; const SIDEBAR_DEFAULT_WIDTH = 280; +const SIDEBAR_MIN_HEIGHT = 120; +const SIDEBAR_MAX_HEIGHT = 400; +const SIDEBAR_DEFAULT_HEIGHT = 180;Then use them in the resize handler and toggle:
- const newHeight = window.innerHeight - e.clientY; - onResize(Math.min(400, Math.max(120, newHeight))); + const newHeight = window.innerHeight - e.clientY; + onResize(Math.min(SIDEBAR_MAX_HEIGHT, Math.max(SIDEBAR_MIN_HEIGHT, newHeight)));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 370 - 371, The bottom-mode uses hard-coded height limits; introduce constants SIDEBAR_MIN_HEIGHT, SIDEBAR_MAX_HEIGHT, and SIDEBAR_DEFAULT_HEIGHT (mirroring SIDEBAR_MIN_WIDTH / SIDEBAR_MAX_WIDTH) and replace the magic numbers in the resize handler (where newHeight is clamped with Math.min/Math.max) and in toggleSidebarPosition (the current hard-coded 180) to use these new constants so all bottom-mode height logic is centralized and consistent.
337-347: Propwidthis semantically misleading — it controls height whenposition === "bottom".The
widthprop is applied as CSSheightfor the bottom variant (line 390) and set to180when toggling to bottom (line 475). Consider renaming it to something generic likesize(andonResize→onResizeSize) or splitting into separatewidth/heightprops so the API is self-documenting.♻️ Suggested rename (prop + state)
- width: number; - onResize: (width: number) => void; + size: number; + onResize: (size: number) => void;And in
App:- const [sidebarWidth, setSidebarWidth] = useState(SIDEBAR_DEFAULT_WIDTH); + const [sidebarSize, setSidebarSize] = useState(SIDEBAR_DEFAULT_WIDTH);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 337 - 347, The prop `width` is misleading because it is used as height when position === "bottom"; rename the prop to `size` and `onResize` to `onResizeSize` across the component and its callers (e.g., the parent App state and toggle logic) so the API is generic: update the prop signature (width → size, onResize → onResizeSize), replace all usages where CSS sets height/width to use `size` and apply it as height when position === "bottom" and as width when position === "side", and update the toggle handler that sets the default 180 to setSize(180) (and rename the state setter accordingly) plus rename any references like onResize(...) → onResizeSize(...). Ensure TypeScript types and all import/prop passthroughs are updated to the new names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend.tsx`:
- Line 359: The declared resizeRef (const resizeRef =
useRef<HTMLDivElement>(null)) is never read; remove the dead code by deleting
the resizeRef declaration and the ref={resizeRef} prop on the resize-handle
<div> (or alternatively, if you actually need bounding-rect access, implement
usage where you call resizeRef.current.getBoundingClientRect() in the relevant
handler); update any imports if useRef becomes unused.
- Around line 473-478: The updater passed to setSidebarPosition (inside
toggleSidebarPosition) performs a side-effect by calling setSidebarWidth, which
violates React's requirement that state updaters be pure; refactor
toggleSidebarPosition so you compute the next position first (using current
state or a functional read if needed) and then call setSidebarPosition and
setSidebarWidth as separate calls outside any updater, or alternatively combine
both into one state object (e.g., sidebarState) and update them together;
reference toggleSidebarPosition, setSidebarPosition, setSidebarWidth and
SIDEBAR_DEFAULT_WIDTH when locating the change.
- Around line 370-371: The bottom-mode uses hard-coded height limits; introduce
constants SIDEBAR_MIN_HEIGHT, SIDEBAR_MAX_HEIGHT, and SIDEBAR_DEFAULT_HEIGHT
(mirroring SIDEBAR_MIN_WIDTH / SIDEBAR_MAX_WIDTH) and replace the magic numbers
in the resize handler (where newHeight is clamped with Math.min/Math.max) and in
toggleSidebarPosition (the current hard-coded 180) to use these new constants so
all bottom-mode height logic is centralized and consistent.
- Around line 337-347: The prop `width` is misleading because it is used as
height when position === "bottom"; rename the prop to `size` and `onResize` to
`onResizeSize` across the component and its callers (e.g., the parent App state
and toggle logic) so the API is generic: update the prop signature (width →
size, onResize → onResizeSize), replace all usages where CSS sets height/width
to use `size` and apply it as height when position === "bottom" and as width
when position === "side", and update the toggle handler that sets the default
180 to setSize(180) (and rename the state setter accordingly) plus rename any
references like onResize(...) → onResizeSize(...). Ensure TypeScript types and
all import/prop passthroughs are updated to the new names.
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
1 participant
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