Improve DiffPanel turn chip strip navigation and scrolling UX#77
Merged
juliusmarminge merged 5 commits intomainfrom Feb 20, 2026
Merged
Improve DiffPanel turn chip strip navigation and scrolling UX#77juliusmarminge merged 5 commits intomainfrom
juliusmarminge merged 5 commits intomainfrom
Conversation
- Add left/right controls, edge fades, and wheel-to-horizontal scrolling for turn chips - Auto-scroll selected turn chip into view and hide the chip strip scrollbar - Simplify chip metadata to show compact per-turn time
WalkthroughReplaces the static "All turns" chip in DiffPanel with a horizontal, scrollable turn-strip containing an All-turns chip and per-turn chips, adds left/right chevrons and wheel-based horizontal scrolling, tracks scroll state with a ref and resize/scroll observers, auto-scrolls/highlights the selected chip, and adds CSS to hide the strip scrollbar and contain overscroll. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
Comment |
Contributor
Improve DiffPanel turn chip strip navigation and scrolling UX in DiffPanel.tsxAdd horizontal scrolling with hidden scrollbars, wheel-to-horizontal mapping, left/right nav buttons, and auto-scroll for the selected chip in DiffPanel.tsx; render compact locale time for chips; add strip styles in index.css. 📍Where to StartStart with the turn strip logic and event handlers in Macroscope summarized f039878. |
Greptile SummaryRefactored
Issue Found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Component Mount] --> B[Initialize turnStripRef]
B --> C[Setup Event Listeners]
C --> D{User Interaction}
D -->|Scroll| E[onScroll Handler]
E --> F[updateTurnStripScrollState]
F --> G[Update canScrollLeft/Right States]
D -->|Vertical Wheel| H[onTurnStripWheel Handler]
H --> I{scrollWidth > clientWidth?}
I -->|Yes| J{abs deltaY > abs deltaX?}
J -->|Yes| K[preventDefault]
K --> L[scrollBy deltaY horizontally]
I -->|No| M[Allow Default]
J -->|No| M
D -->|Click Chevron| N[scrollTurnStripBy offset]
N --> O[element.scrollBy with smooth]
O --> E
D -->|Turn Selection Change| P[Auto-scroll Effect]
P --> Q[Find selected chip]
Q --> R[scrollIntoView smooth]
S[ResizeObserver] --> F
T[Component Unmount] --> U[Cleanup Listeners]
U --> V[Disconnect ResizeObserver]
Last reviewed commit: c00354c |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 82-87: The formatTurnChipTimestamp function is calling
Intl.DateTimeFormat(...).format(new Date(isoDate)) without guarding against
invalid dates; update formatTurnChipTimestamp to validate the parsed date (e.g.,
const d = new Date(isoDate); if (Number.isNaN(d.getTime())) return a safe
fallback like an empty string or "-" ) and only call Intl.DateTimeFormat.format
when the date is valid. Keep the function name formatTurnChipTimestamp and
ensure callers (e.g., deriveTurnDiffSummaries and any places using completedAt)
continue to receive a stable string even if the timestamp is malformed.
Comment on lines
+82
to
+87
| function formatTurnChipTimestamp(isoDate: string): string { | ||
| return new Intl.DateTimeFormat(undefined, { | ||
| hour: "numeric", | ||
| minute: "2-digit", | ||
| }).format(new Date(isoDate)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "useTurnDiffSummaries|TurnDiffSummary|completedAt" --type=ts --type=tsx -C 3Repository: pingdotgg/codething-mvp
Length of output: 94
🏁 Script executed:
# Also check the DiffPanel.tsx file to see the full context around formatTurnChipTimestamp
head -120 apps/web/src/components/DiffPanel.tsx | tail -50Repository: pingdotgg/codething-mvp
Length of output: 1594
🏁 Script executed:
# Search for Zod schema definitions in packages/contracts
find packages/contracts -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: pingdotgg/codething-mvp
Length of output: 727
🏁 Script executed:
# Search for TurnDiffSummary, useTurnDiffSummaries, completedAt without type flags
rg "TurnDiffSummary|useTurnDiffSummaries|completedAt" -C 3Repository: pingdotgg/codething-mvp
Length of output: 21964
🏁 Script executed:
# Check DiffPanel.tsx to see how formatTurnChipTimestamp is called
rg "formatTurnChipTimestamp" -C 5Repository: pingdotgg/codething-mvp
Length of output: 1731
🏁 Script executed:
# Look for provider event schema in ws.ts and provider.ts
cat packages/contracts/src/ws.tsRepository: pingdotgg/codething-mvp
Length of output: 3163
Guard against invalid completedAt timestamps to avoid runtime errors.
While the current code path guarantees completedAt is a non-empty string (due to filtering in deriveTurnDiffSummaries and Zod validation in persistence), Intl.DateTimeFormat(...).format(new Date(isoDate)) remains unguarded and vulnerable to invalid dates. Add a safe fallback for consistency with defensive date-parsing patterns used elsewhere in the codebase (e.g., store.ts, Sidebar.tsx).
🛡️ Proposed guard
function formatTurnChipTimestamp(isoDate: string): string {
- return new Intl.DateTimeFormat(undefined, {
- hour: "numeric",
- minute: "2-digit",
- }).format(new Date(isoDate));
+ const date = new Date(isoDate);
+ if (Number.isNaN(date.getTime())) return "--";
+ return new Intl.DateTimeFormat(undefined, {
+ hour: "numeric",
+ minute: "2-digit",
+ }).format(date);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/DiffPanel.tsx` around lines 82 - 87, The
formatTurnChipTimestamp function is calling Intl.DateTimeFormat(...).format(new
Date(isoDate)) without guarding against invalid dates; update
formatTurnChipTimestamp to validate the parsed date (e.g., const d = new
Date(isoDate); if (Number.isNaN(d.getTime())) return a safe fallback like an
empty string or "-" ) and only call Intl.DateTimeFormat.format when the date is
valid. Keep the function name formatTurnChipTimestamp and ensure callers (e.g.,
deriveTurnDiffSummaries and any places using completedAt) continue to receive a
stable string even if the timestamp is malformed.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 253-260: The wheel handler onTurnStripWheel currently calls
event.preventDefault() but React's onWheel is passive so preventDefault() is
ignored; update the component to attach the listener directly to the DOM node
referenced by turnStripRef using element.addEventListener('wheel', handler, {
passive: false }) inside a useEffect (and remove the onWheel prop if present),
implement the same logic as onTurnStripWheel in that handler (check
turnStripRef.current, scrollWidth <= clientWidth + 1, compare deltaY/deltaX,
call preventDefault(), call element.scrollBy), and ensure you return a cleanup
that removes the event listener to avoid leaks.
---
Duplicate comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 77-82: The function formatTurnChipTimestamp currently calls new
Date(isoDate) and formats it without checking validity; update
formatTurnChipTimestamp to construct the Date once, check its validity (e.g.,
isNaN(date.getTime()) or !isFinite(date.getTime())), and if invalid return a
safe fallback (empty string or a placeholder) instead of calling
Intl.DateTimeFormat.format on an invalid date; otherwise proceed to format the
valid date as before.
Member
Author
|
@greptileai review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/DiffPanel.tsx (1)
279-285: Consider simplifying the effect dependencies.The effect depends on both
selectedTurn?.turnIdandselectedTurnId, butselectedTurnis derived fromselectedTurnId. One dependency should suffice.♻️ Optional simplification
useEffect(() => { const element = turnStripRef.current; if (!element) return; const selectedChip = element.querySelector<HTMLElement>("[data-turn-chip-selected='true']"); selectedChip?.scrollIntoView({ block: "nearest", inline: "nearest", behavior: "smooth" }); - }, [selectedTurn?.turnId, selectedTurnId]); + }, [selectedTurnId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/DiffPanel.tsx` around lines 279 - 285, The useEffect in DiffPanel that scrolls the selected chip uses both selectedTurn?.turnId and selectedTurnId even though selectedTurn is derived from selectedTurnId; remove the redundant dependency and keep only the source of truth (e.g., selectedTurnId) in the dependency array. Update the effect declaration that references turnStripRef and selectedChip (inside the useEffect callback) to depend solely on selectedTurnId (or, if you prefer, on selectedTurn but then remove selectedTurnId) so the effect triggers correctly without duplicate/derived dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 70-75: The formatTurnChipTimestamp function can produce "Invalid
Date" for bad or missing timestamps; update formatTurnChipTimestamp(isoDate:
string) to validate the parsed Date (e.g., check isNaN(date.getTime()) or
Number.isNaN(+date)) before formatting and return a safe fallback (empty string
or a localized "Unknown") when invalid; ensure callers that pass completedAt
rely on this safe fallback so the UI never displays "Invalid Date".
---
Nitpick comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 279-285: The useEffect in DiffPanel that scrolls the selected chip
uses both selectedTurn?.turnId and selectedTurnId even though selectedTurn is
derived from selectedTurnId; remove the redundant dependency and keep only the
source of truth (e.g., selectedTurnId) in the dependency array. Update the
effect declaration that references turnStripRef and selectedChip (inside the
useEffect callback) to depend solely on selectedTurnId (or, if you prefer, on
selectedTurn but then remove selectedTurnId) so the effect triggers correctly
without duplicate/derived dependencies.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/DiffPanel.tsx (1)
246-254: Remove unusedonTurnStripWheelcallback.This callback is defined but never used. The wheel event handler is defined inline within the
useEffectat lines 263-269 with identical logic. Remove this dead code to avoid duplication and confusion.🧹 Remove dead code
const scrollTurnStripBy = useCallback((offset: number) => { const element = turnStripRef.current; if (!element) return; element.scrollBy({ left: offset, behavior: "smooth" }); }, []); - const onTurnStripWheel = useCallback((event: WheelEvent) => { - const element = turnStripRef.current; - if (!element) return; - if (element.scrollWidth <= element.clientWidth + 1) return; - if (Math.abs(event.deltaY) <= Math.abs(event.deltaX)) return; - - event.preventDefault(); - element.scrollBy({ left: event.deltaY, behavior: "auto" }); - }, []); useEffect(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/DiffPanel.tsx` around lines 246 - 254, Remove the unused dead callback onTurnStripWheel: it's defined with turnStripRef but never referenced and duplicates the inline wheel handler inside the useEffect; delete the onTurnStripWheel definition and any imports/hooks solely used for it so the component uses only the single inline wheel event logic in the useEffect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 70-75: The function formatTurnChipTimestamp should validate the
incoming ISO string before formatting to guard against malformed completedAt
values: in formatTurnChipTimestamp(isoDate) parse the string into a Date, check
that date.getTime() is a valid number (not NaN), and if invalid return a safe
fallback (e.g., empty string or a clear placeholder) instead of calling
Intl.DateTimeFormat; otherwise proceed to format and return the localized time.
Ensure you reference formatTurnChipTimestamp and the completedAt input when
adding this guard so callers receive a stable value for malformed timestamps.
---
Nitpick comments:
In `@apps/web/src/components/DiffPanel.tsx`:
- Around line 246-254: Remove the unused dead callback onTurnStripWheel: it's
defined with turnStripRef but never referenced and duplicates the inline wheel
handler inside the useEffect; delete the onTurnStripWheel definition and any
imports/hooks solely used for it so the component uses only the single inline
wheel event logic in the useEffect.
Contributor
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: Preview (432668d253)diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx
--- a/apps/web/src/components/DiffPanel.tsx
+++ b/apps/web/src/components/DiffPanel.tsx
@@ -260,26 +260,18 @@
updateTurnStripScrollState();
const onScroll = () => updateTurnStripScrollState();
- const onWheel = (event: WheelEvent) => {
- if (!element) return;
- if (element.scrollWidth <= element.clientWidth + 1) return;
- if (Math.abs(event.deltaY) <= Math.abs(event.deltaX)) return;
- event.preventDefault();
- element.scrollBy({ left: event.deltaY, behavior: "auto" });
- };
-
element.addEventListener("scroll", onScroll, { passive: true });
- element.addEventListener("wheel", onWheel, { passive: false });
+ element.addEventListener("wheel", onTurnStripWheel, { passive: false });
const resizeObserver = new ResizeObserver(() => updateTurnStripScrollState());
resizeObserver.observe(element);
return () => {
element.removeEventListener("scroll", onScroll);
- element.removeEventListener("wheel", onWheel);
+ element.removeEventListener("wheel", onTurnStripWheel);
resizeObserver.disconnect();
};
- }, [updateTurnStripScrollState]);
+ }, [updateTurnStripScrollState, onTurnStripWheel]);
useEffect(() => {
updateTurnStripScrollState(); |
Member
Author
…useCallback in useEffect Applied via @cursor push command
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| Turn {turnDiffSummaries.length - index} | ||
| </span> | ||
| <span className="text-[9px] leading-tight opacity-70"> | ||
| {formatTurnChipTimestamp(summary.completedAt)} |
Contributor
There was a problem hiding this comment.
🟡 Medium
components/DiffPanel.tsx:383 formatTurnChipTimestamp can throw if summary.completedAt is invalid. Consider adding a guard similar to inferCheckpointTurnCountByTurnId that checks Number.isNaN(Date.parse(...)) before formatting.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/components/DiffPanel.tsx around line 383:
`formatTurnChipTimestamp` can throw if `summary.completedAt` is invalid. Consider adding a guard similar to `inferCheckpointTurnCountByTurnId` that checks `Number.isNaN(Date.parse(...))` before formatting.
Evidence trail:
apps/web/src/components/DiffPanel.tsx lines 70-75 (formatTurnChipTimestamp function definition - no validation), line 383 (usage site). apps/web/src/session-logic.ts lines 571-581 (inferCheckpointTurnCountByTurnId with Number.isNaN guard at line 581). Commit f039878edd64cac3cd7b3e5d95d41e228544b3aa.
jjalangtry
pushed a commit
to jjalangtry/t3code
that referenced
this pull request
Mar 16, 2026
Improve DiffPanel turn chip strip navigation and scrolling UX
smraikai
pushed a commit
to smraikai/t3code
that referenced
this pull request
Apr 16, 2026
Improve DiffPanel turn chip strip navigation and scrolling UX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.turn-chip-stripstyles.Testing
Note
Low Risk
UI-only changes to diff navigation/scroll behavior; primary risk is minor usability regressions or event-handler edge cases across browsers/Electron.
Overview
Refactors the DiffPanel turn selector into a dedicated horizontal chip strip with left/right chevron controls, overflow fade indicators, and scroll-state tracking to enable/disable navigation buttons.
Adds wheel-to-horizontal scrolling, auto-scrolls the selected chip into view, and switches turn timestamps to a compact time-only format. Updates CSS with
.turn-chip-stripstyles to hide scrollbars and contain horizontal overscroll.Written by Cursor Bugbot for commit f039878. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Style