Resizeable panels#14
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe app adds an accessible, pointer-driven horizontal split between the upper panels and timeline. Timeline height is clamped, persisted, restored during startup, reset by double-click, and reclamped when the viewport changes. ChangesTimeline panel resizing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant splitUpperTimeline
participant initPanelSplit
participant localStorage
participant timelinePanel
User->>splitUpperTimeline: Drag handle
splitUpperTimeline->>initPanelSplit: Update split height
initPanelSplit->>timelinePanel: Apply clamped --timeline-h
initPanelSplit->>localStorage: Save height
User->>splitUpperTimeline: Double-click to reset
initPanelSplit->>timelinePanel: Apply default height
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
index.html (1)
95-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding ARIA value attributes and
tabindexfor accessibility.The
role="separator"is correct, but resizable separators benefit fromaria-valuenow,aria-valuemin,aria-valuemax, andtabindex="0"so screen reader and keyboard users can perceive and adjust the split. Since the PR is pointer-driven, this can be deferred, but the ARIA values are low-cost to add.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@index.html` at line 95, Update the splitUpperTimeline separator element to include keyboard focusability via tabindex="0" and expose its current, minimum, and maximum split values with aria-valuenow, aria-valuemin, and aria-valuemax, keeping those values synchronized with the resizable split state.app.js (1)
2863-2883: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache
availableTimelineMax()at drag start to avoid per-move reflows.
setTimelineHeightcallsavailableTimelineMax()on everypointermove, triggering twogetBoundingClientRect()calls (forced reflows) per move. Since the available max doesn't change during a drag, compute it once inpointerdownand pass it through.⚡ Proposed changes
Add an optional max override to
setTimelineHeight:-function setTimelineHeight(px) { - const h = clamp(Math.round(px), TL_H_MIN, Math.max(TL_H_MIN, availableTimelineMax())); +function setTimelineHeight(px, maxOverride) { + const max = maxOverride ?? Math.max(TL_H_MIN, availableTimelineMax()); + const h = clamp(Math.round(px), TL_H_MIN, max); $("app").style.setProperty("--timeline-h", h + "px"); state.dirtyTimeline = true; return h; }Then use the cached value in the drag handler:
const y0 = e.clientY; const h0 = tl.getBoundingClientRect().height; - const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0)); + const maxH = Math.max(TL_H_MIN, availableTimelineMax()); + const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0), maxH);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app.js` around lines 2863 - 2883, Update setTimelineHeight to accept an optional cached maximum-height override and use it instead of calling availableTimelineMax() when provided. In the handle pointerdown flow, compute availableTimelineMax() once at drag start, capture it, and pass it to setTimelineHeight from the pointermove handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app.js`:
- Around line 2831-2838: Update availableTimelineMax to subtract the app’s 12px
total vertical padding from the getBoundingClientRect height before calculating
the available timeline maximum, preserving the existing topbar, minimum, split,
and gap deductions.
- Around line 2863-2883: Update the pointer drag handling around the pointerdown
listener by extracting the shared cleanup from onUp into a reusable handler,
then register it for both pointerup and pointercancel. Ensure cancellation
releases the pointer capture, removes dragging/resizing-panels classes,
unregisters move and end listeners, and preserves the existing persistence and
dirty-state behavior for normal pointerup.
---
Nitpick comments:
In `@app.js`:
- Around line 2863-2883: Update setTimelineHeight to accept an optional cached
maximum-height override and use it instead of calling availableTimelineMax()
when provided. In the handle pointerdown flow, compute availableTimelineMax()
once at drag start, capture it, and pass it to setTimelineHeight from the
pointermove handler.
In `@index.html`:
- Line 95: Update the splitUpperTimeline separator element to include keyboard
focusability via tabindex="0" and expose its current, minimum, and maximum split
values with aria-valuenow, aria-valuemin, and aria-valuemax, keeping those
values synchronized with the resizable split state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c40c514-deeb-4e7a-b75e-5b2454f00b39
📒 Files selected for processing (3)
app.jsindex.htmlstyle.css
Comment on lines
+2831
to
+2838
| function availableTimelineMax() { | ||
| const app = $("app").getBoundingClientRect(); | ||
| const topbar = document.querySelector(".topbar")?.getBoundingClientRect(); | ||
| const topbarH = topbar ? topbar.height : 46; | ||
| const split = 6; | ||
| const gaps = 18; // three 6px flex gaps between four children | ||
| return Math.floor(app.height - topbarH - UPPER_MIN - split - gaps); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
availableTimelineMax doesn't account for .app's 12px vertical padding.
getBoundingClientRect().height includes padding, but flex children are laid out in the content area (height - padding). The calculation is missing 12px (6px top + 6px bottom), so the max is overestimated by 12px, causing slight overflow when the timeline is dragged to its maximum.
🐛 Proposed fix
function availableTimelineMax() {
const app = $("app").getBoundingClientRect();
const topbar = document.querySelector(".topbar")?.getBoundingClientRect();
const topbarH = topbar ? topbar.height : 46;
const split = 6;
const gaps = 18; // three 6px flex gaps between four children
- return Math.floor(app.height - topbarH - UPPER_MIN - split - gaps);
+ const padding = 12; // 6px top + 6px bottom on .app
+ return Math.floor(app.height - padding - topbarH - UPPER_MIN - split - gaps);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function availableTimelineMax() { | |
| const app = $("app").getBoundingClientRect(); | |
| const topbar = document.querySelector(".topbar")?.getBoundingClientRect(); | |
| const topbarH = topbar ? topbar.height : 46; | |
| const split = 6; | |
| const gaps = 18; // three 6px flex gaps between four children | |
| return Math.floor(app.height - topbarH - UPPER_MIN - split - gaps); | |
| } | |
| function availableTimelineMax() { | |
| const app = $("app").getBoundingClientRect(); | |
| const topbar = document.querySelector(".topbar")?.getBoundingClientRect(); | |
| const topbarH = topbar ? topbar.height : 46; | |
| const split = 6; | |
| const gaps = 18; // three 6px flex gaps between four children | |
| const padding = 12; // 6px top + 6px bottom on .app | |
| return Math.floor(app.height - padding - topbarH - UPPER_MIN - split - gaps); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app.js` around lines 2831 - 2838, Update availableTimelineMax to subtract the
app’s 12px total vertical padding from the getBoundingClientRect height before
calculating the available timeline maximum, preserving the existing topbar,
minimum, split, and gap deductions.
Comment on lines
+2863
to
+2883
| handle.addEventListener("pointerdown", (e) => { | ||
| if (e.button !== 0) return; | ||
| e.preventDefault(); | ||
| handle.setPointerCapture(e.pointerId); | ||
| handle.classList.add("dragging"); | ||
| document.body.classList.add("resizing-panels"); | ||
| const y0 = e.clientY; | ||
| const h0 = tl.getBoundingClientRect().height; | ||
| const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0)); | ||
| const onUp = () => { | ||
| handle.releasePointerCapture(e.pointerId); | ||
| handle.classList.remove("dragging"); | ||
| document.body.classList.remove("resizing-panels"); | ||
| handle.removeEventListener("pointermove", onMove); | ||
| handle.removeEventListener("pointerup", onUp); | ||
| localStorage.setItem(TL_H_KEY, String(Math.round(tl.getBoundingClientRect().height))); | ||
| state.dirtyTimeline = true; | ||
| }; | ||
| handle.addEventListener("pointermove", onMove); | ||
| handle.addEventListener("pointerup", onUp); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a pointercancel handler to clean up drag state.
If the browser cancels the pointer (common on touch when the OS or a scroll gesture intervenes), pointerup never fires. The dragging class on the handle and resizing-panels class on <body> remain stuck, leaving cursor: ns-resize and user-select: none on all elements until page reload. Extract the shared cleanup and register it for pointercancel as well.
🔒 Proposed fix
const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0));
- const onUp = () => {
+ const cleanup = () => {
handle.releasePointerCapture(e.pointerId);
handle.classList.remove("dragging");
document.body.classList.remove("resizing-panels");
handle.removeEventListener("pointermove", onMove);
handle.removeEventListener("pointerup", onUp);
+ handle.removeEventListener("pointercancel", cleanup);
+ };
+ const onUp = () => {
+ cleanup();
localStorage.setItem(TL_H_KEY, String(Math.round(tl.getBoundingClientRect().height)));
state.dirtyTimeline = true;
};
handle.addEventListener("pointermove", onMove);
handle.addEventListener("pointerup", onUp);
+ handle.addEventListener("pointercancel", cleanup);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| handle.addEventListener("pointerdown", (e) => { | |
| if (e.button !== 0) return; | |
| e.preventDefault(); | |
| handle.setPointerCapture(e.pointerId); | |
| handle.classList.add("dragging"); | |
| document.body.classList.add("resizing-panels"); | |
| const y0 = e.clientY; | |
| const h0 = tl.getBoundingClientRect().height; | |
| const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0)); | |
| const onUp = () => { | |
| handle.releasePointerCapture(e.pointerId); | |
| handle.classList.remove("dragging"); | |
| document.body.classList.remove("resizing-panels"); | |
| handle.removeEventListener("pointermove", onMove); | |
| handle.removeEventListener("pointerup", onUp); | |
| localStorage.setItem(TL_H_KEY, String(Math.round(tl.getBoundingClientRect().height))); | |
| state.dirtyTimeline = true; | |
| }; | |
| handle.addEventListener("pointermove", onMove); | |
| handle.addEventListener("pointerup", onUp); | |
| }); | |
| handle.addEventListener("pointerdown", (e) => { | |
| if (e.button !== 0) return; | |
| e.preventDefault(); | |
| handle.setPointerCapture(e.pointerId); | |
| handle.classList.add("dragging"); | |
| document.body.classList.add("resizing-panels"); | |
| const y0 = e.clientY; | |
| const h0 = tl.getBoundingClientRect().height; | |
| const onMove = (ev) => setTimelineHeight(h0 - (ev.clientY - y0)); | |
| const cleanup = () => { | |
| handle.releasePointerCapture(e.pointerId); | |
| handle.classList.remove("dragging"); | |
| document.body.classList.remove("resizing-panels"); | |
| handle.removeEventListener("pointermove", onMove); | |
| handle.removeEventListener("pointerup", onUp); | |
| handle.removeEventListener("pointercancel", cleanup); | |
| }; | |
| const onUp = () => { | |
| cleanup(); | |
| localStorage.setItem(TL_H_KEY, String(Math.round(tl.getBoundingClientRect().height))); | |
| state.dirtyTimeline = true; | |
| }; | |
| handle.addEventListener("pointermove", onMove); | |
| handle.addEventListener("pointerup", onUp); | |
| handle.addEventListener("pointercancel", cleanup); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app.js` around lines 2863 - 2883, Update the pointer drag handling around the
pointerdown listener by extracting the shared cleanup from onUp into a reusable
handler, then register it for both pointerup and pointercancel. Ensure
cancellation releases the pointer capture, removes dragging/resizing-panels
classes, unregisters move and end listeners, and preserves the existing
persistence and dirty-state behavior for normal pointerup.
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.
What does this PR do?
Make monitor/timeline panes resizeable. There is a handle between panes that allows to resize, double click restores the default layout
Closes #
Type of change
How was it verified?
node --check server.js && node --check app.js && node --check mcp-server.jspassesCLAUDE.md/README.mdif the schema, props, or API changedChecklist
Summary by CodeRabbit