feat(ui): chat visualization + minimap rail#203
feat(ui): chat visualization + minimap rail#203benjaminshafii merged 16 commits intodifferent-ai:devfrom
Conversation
- Update SessionView to accept sessionStatusById prop - Render status pill in session list sidebar - Pass status from App component to SessionView - Fix type errors in demo-state mocks
- Add 'flow' animation for new tasks, artifacts, and files moving to sidebar - Add minimalist jump rail to navigate between Agent and User messages - Add copy button to message bubbles - Add OnePlus-inspired colors for User interactions
- Replaces simple jump buttons with a minimap rail showing individual message lines - Uses #EB0029 for user messages and theme-inverse for agent messages - Implements active state highlighting and hover expansion effects - Adds click-to-scroll navigation for each message line
- Change lines from vertical to horizontal dashes - Remove rail border for cleaner look - Unify line thickness (2px default, 3px active) - Enhance active state with expansion and glow effect - Improve clickability with expanded hit areas - Use #EB0029 for user messages and theme-inverse for agent
- Add no-scrollbar utility class and CSS rules - Ensure clean look with only the custom minimap rail visible
- Default width: 2.5 (was 1.5) - Active width: 4 (was 3) - Hover width: 3.5 (was 2.5) - Keeps the same premium thin height
|
The following comment was made by an LLM, it may be inaccurate: |
|
There was a problem hiding this comment.
Pull request overview
This PR adds visual enhancements to the session chat interface, introducing a minimap-style navigation rail, animated flyouts for item creation, session status badges, and message copy functionality. The changes enhance user experience by providing better navigation and visual feedback for long chat sessions.
Changes:
- Added per-message minimap rail for chat navigation with clickable markers color-coded by role (user/assistant)
- Implemented animated flyout items that show when tasks, artifacts, or files are created and fly to their sidebar destinations
- Added session status badges in the workspace sidebar showing running/idle states with visual indicators
- Integrated copy buttons for messages that appear on hover
- Hidden native scrollbar in favor of the minimap rail
- Updated demo state to include required
workspaceTypefield for workspace displays
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| packages/app/src/app/pages/session.tsx | Main changes including minimap rail, flyout animations, copy buttons, and session status badges |
| packages/app/src/app/demo-state.ts | Added required workspaceType: "local" field to demo workspace displays |
| packages/app/src/app/app.tsx | Passed sessionStatusById prop to SessionView component |
| packages/app/pr/screenshots/minimap-rail-2.png | Screenshot showing new minimap rail feature (binary file) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const handleCopy = async (text: string, id: string) => { | ||
| try { | ||
| await navigator.clipboard.writeText(text); | ||
| setCopyingId(id); | ||
| setTimeout(() => setCopyingId(null), 2000); |
There was a problem hiding this comment.
The copy button's success state timeout (2000ms) is not cleaned up if the component unmounts or if another copy operation starts before the timeout completes. This could lead to the wrong message showing a success state or attempting to update state after unmount. Consider clearing the timeout when a new copy starts or on component cleanup.
| const handleCopy = async (text: string, id: string) => { | |
| try { | |
| await navigator.clipboard.writeText(text); | |
| setCopyingId(id); | |
| setTimeout(() => setCopyingId(null), 2000); | |
| let copyTimeout: number | undefined; | |
| onCleanup(() => { | |
| if (copyTimeout !== undefined) { | |
| clearTimeout(copyTimeout); | |
| } | |
| }); | |
| const handleCopy = async (text: string, id: string) => { | |
| try { | |
| await navigator.clipboard.writeText(text); | |
| setCopyingId(id); | |
| if (copyTimeout !== undefined) { | |
| clearTimeout(copyTimeout); | |
| } | |
| copyTimeout = window.setTimeout(() => { | |
| setCopyingId(null); | |
| }, 2000); |
| createEffect(() => { | ||
| // Re-calc lines when messages change | ||
| props.messages.length; | ||
| // Wait for render | ||
| setTimeout(updateMessageLines, 100); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The createEffect that triggers minimap line recalculation when messages change uses a hardcoded 100ms delay via setTimeout. This arbitrary delay could cause visual inconsistencies if messages render faster or slower than expected. Consider using a more robust approach like monitoring the actual DOM changes with a MutationObserver, or at minimum, using requestAnimationFrame instead of an arbitrary timeout.
| createEffect(() => { | |
| // Re-calc lines when messages change | |
| props.messages.length; | |
| // Wait for render | |
| setTimeout(updateMessageLines, 100); | |
| }); | |
| let messageLinesRafId: number | null = null; | |
| createEffect(() => { | |
| // Re-calc lines when messages change | |
| props.messages.length; | |
| if (messageLinesRafId !== null) { | |
| cancelAnimationFrame(messageLinesRafId); | |
| } | |
| messageLinesRafId = requestAnimationFrame(() => { | |
| updateMessageLines(); | |
| messageLinesRafId = null; | |
| }); | |
| }); | |
| onCleanup(() => { | |
| if (messageLinesRafId !== null) { | |
| cancelAnimationFrame(messageLinesRafId); | |
| } | |
| }); |
| ) => { | ||
| if (isInitialLoad() || !sourceEl) return; | ||
| const targetEl = document.getElementById(targetId); | ||
| if (!targetEl) return; |
There was a problem hiding this comment.
The flyout animations query the DOM using document.querySelector and document.getElementById which could fail if the target elements haven't rendered yet or have been removed. The code handles the null case for the source element, but if the target element isn't found, triggerFlyout will silently return early. Consider adding error logging or user feedback when flyout animations fail to help with debugging.
| if (!targetEl) return; | |
| if (!targetEl) { | |
| console.warn( | |
| "[session] triggerFlyout: target element not found", | |
| { targetId, label } | |
| ); | |
| return; | |
| } |
| <div class="hidden lg:flex w-5 bg-transparent flex-col items-center justify-start relative group/rail z-10 overflow-hidden py-1"> | ||
| <For each={messageLines()}> | ||
| {(line) => ( | ||
| <div | ||
| class={`absolute left-1/2 -translate-x-1/2 rounded-full transition-all duration-300 ease-out cursor-pointer | ||
| ${ | ||
| line.id === activeMessageId() | ||
| ? "w-4 h-[3px] opacity-100 z-20 shadow-[0_0_8px_rgba(235,0,41,0.6)] dark:shadow-[0_0_8px_rgba(255,255,255,0.6)]" | ||
| : "w-2.5 h-[2px] opacity-30 hover:opacity-80 hover:w-3.5" | ||
| } | ||
| `} | ||
| style={{ | ||
| top: `${line.top}px`, | ||
| "background-color": line.role === "user" ? "#EB0029" : "var(--rail-agent-color, currentColor)", | ||
| }} | ||
| title={line.role === "user" ? "User" : "Agent"} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| const el = chatContainerEl?.querySelector(`[data-message-id="${line.id}"]`); | ||
| el?.scrollIntoView({ behavior: "smooth", block: "center" }); | ||
| }} | ||
| > | ||
| {/* Invisible hit area for easier clicking */} | ||
| <div class="absolute -inset-x-2 -inset-y-1 bg-transparent" /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
The minimap rail lacks proper accessibility features. The interactive rail markers should have proper ARIA labels, keyboard navigation support (tab through markers, Enter/Space to activate), and focus indicators. Currently, the markers are only mouse-clickable divs without keyboard accessibility, which makes the feature unusable for keyboard-only users. Consider wrapping each marker in a button element with appropriate ARIA attributes and keyboard event handlers.
| <style> | ||
| {` | ||
| .no-scrollbar::-webkit-scrollbar { | ||
| display: none; | ||
| } | ||
| .no-scrollbar { | ||
| -ms-overflow-style: none; | ||
| scrollbar-width: none; | ||
| } | ||
| `} | ||
| </style> |
There was a problem hiding this comment.
Hiding the native scrollbar with the no-scrollbar class removes an important visual indicator for users about the scroll position and scrollable content length. While the minimap rail provides some indication, it's less intuitive than a traditional scrollbar. Consider adding a visual indicator on the minimap rail that shows the current viewport position within the total content height (similar to how code editors display visible range indicators on their minimaps).
| <style> | ||
| {` | ||
| :root { --rail-agent-color: #000; } | ||
| [data-theme="dark"] { --rail-agent-color: #fff; } | ||
| `} | ||
| </style> |
There was a problem hiding this comment.
The minimap rail uses inline styles within a <style> JSX element to define CSS custom properties. This creates duplicate <style> elements in the DOM - one for hiding scrollbars (lines 1090-1100) and another for the rail colors (lines 1334-1339). These should be consolidated into a single style block or moved to a global stylesheet to avoid DOM pollution and improve performance.
| }} | ||
| > | ||
| {/* Invisible hit area for easier clicking */} | ||
| <div class="absolute -inset-x-2 -inset-y-1 bg-transparent" /> |
There was a problem hiding this comment.
The minimap rail click handler creates invisible hit areas for easier clicking, but these use negative insets which could potentially overlap with adjacent elements or cause unexpected click behavior. The hit area extends 8px horizontally (-inset-x-2 = -0.5rem = -8px) and 4px vertically (-inset-y-1 = -0.25rem = -4px) beyond the visible rail markers. Given that the rail itself is only 20px wide (w-5), this could cause unintended interactions. Consider reducing the hit area size or ensuring proper z-index layering.
| <div class="absolute -inset-x-2 -inset-y-1 bg-transparent" /> | |
| <div class="absolute inset-x-0 -inset-y-1 bg-transparent" /> |
| // Debounced update on scroll | ||
| const handleScroll = () => { | ||
| requestAnimationFrame(updateMessageLines); |
There was a problem hiding this comment.
The handleScroll function uses requestAnimationFrame for debouncing, but this provides no actual debouncing - it will still call updateMessageLines on every scroll event (just on the next animation frame). For performance, especially with long chat histories, consider implementing proper throttling to limit how frequently the minimap updates during scrolling. A throttle interval of 100-150ms would be appropriate here.
| // Debounced update on scroll | |
| const handleScroll = () => { | |
| requestAnimationFrame(updateMessageLines); | |
| // Throttled update on scroll to avoid expensive recalculations on every scroll event | |
| const THROTTLE_INTERVAL = 120; // milliseconds | |
| let lastUpdateTime = 0; | |
| let throttleTimeout: number | null = null; | |
| const throttledUpdateMessageLines = () => { | |
| const now = Date.now(); | |
| const remaining = THROTTLE_INTERVAL - (now - lastUpdateTime); | |
| if (remaining <= 0) { | |
| lastUpdateTime = now; | |
| updateMessageLines(); | |
| } else { | |
| if (throttleTimeout !== null) { | |
| clearTimeout(throttleTimeout); | |
| } | |
| throttleTimeout = window.setTimeout(() => { | |
| lastUpdateTime = Date.now(); | |
| updateMessageLines(); | |
| throttleTimeout = null; | |
| }, remaining); | |
| } | |
| }; | |
| const handleScroll = () => { | |
| throttledUpdateMessageLines(); |
| setTimeout(() => { | ||
| const card = document.querySelector(`[data-artifact-id="${last.id}"]`); | ||
| triggerFlyout(card, "sidebar-artifacts", last.name, "file"); | ||
| }, 100); | ||
| } |
There was a problem hiding this comment.
The artifact flyout animation uses a setTimeout delay of 100ms before querying for the artifact card element. This is a race condition - there's no guarantee that the artifact card will have rendered and be in the DOM after exactly 100ms. If rendering is slower (e.g., on a slow device), the flyout animation will fail silently. Consider using a more reliable approach like a ref callback or checking for the element's existence with retry logic.
| const rect = sourceEl.getBoundingClientRect(); | ||
| const targetRect = targetEl.getBoundingClientRect(); | ||
|
|
||
| const id = Math.random().toString(36); |
There was a problem hiding this comment.
Using Math.random().toString(36) for generating IDs is not cryptographically secure and could potentially generate duplicate IDs, though the probability is low. Consider using crypto.randomUUID() if available, or ensure the ID generation includes additional uniqueness factors like a timestamp.
- debounce minimap line updates with raf scheduler - clean up timeouts and rafs on unmount - add retry logic for artifact flyout source lookup - ensure minimap markers are keyboard accessible
|
thx @Golenspade I added a few changes in styling and will merge the branch soon. |
* feat: enable windows sidecar bundling * fix: run sidecar prep from repo root * fix: run sidecar prep via workspace filter
|
@benjaminshafii thx |
|
what do you mean by "the point"? are you asking about how we decide to design things? |
Summary
Testing
Screenshots