feat: scope tree helper hover highlight + wider clickable rails#61
Closed
dpshde wants to merge 1 commit into
Closed
feat: scope tree helper hover highlight + wider clickable rails#61dpshde wants to merge 1 commit into
dpshde wants to merge 1 commit into
Conversation
… entire column) - Use ownerId to tag only descendant rows for .rail-hovered - Highlight only the relevant segments under the hovered ancestor - Keep normal guides flat Also includes supporting railOwnerIds propagation and e2e updates.
📝 WalkthroughWalkthroughReplaces row chevron collapse affordances with hoverable ancestor guide rails: ChangesGuide Rail Interaction Feature
Auth Bypass Flag
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RailToggles
participant OutlineNode
participant OutlineRow
User->>RailToggles: mouseover rail button
RailToggles->>OutlineNode: set --hovered-rail-x, add rail-hovered class
User->>RailToggles: click rail toggle
RailToggles->>OutlineRow: toggle collapsed state
OutlineRow-->>OutlineNode: re-render collapsed/expanded rows
User->>RailToggles: mouseleave list
RailToggles->>OutlineNode: remove rail-hovered class
Possibly related PRs
🚥 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/routes/__root.tsx (1)
85-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGuard the auth bypass behind dev-only builds.
VITE_BYPASS_AUTHskips the session gate entirely, and becauseVITE_env vars are baked into the client bundle, a stray staging/prod build setting would expose the app without auth. Restrict this toimport.meta.env.DEV(or another explicit non-prod check) so the bypass can’t be enabled in shipped builds.🤖 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 `@src/routes/__root.tsx` around lines 85 - 91, The AuthGate bypass currently relies only on VITE_BYPASS_AUTH, which can be baked into shipped client builds; update AuthGate to allow the bypass only when import.meta.env.DEV (or an equivalent explicit non-production check) is also true, so the session gate cannot be skipped in staging/prod. Keep the change localized to the AuthGate logic and preserve the existing useSession, isPending, and AuthScreen flow for normal authenticated access.src/components/OutlineNode.tsx (1)
293-304: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecursive fallback needs a visible collapse affordance.
The non-virtualized path still uses the invisible
.collapse-toggle, but it never gets the rail-based controls the windowed path uses (data-depth/rail toggles), so collapse/expand is only hinted at by the subtle collapsed-bullet styling. That makes the fallback harder to discover whenvirtualizedis off.🤖 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 `@src/components/OutlineNode.tsx` around lines 293 - 304, The non-virtualized row in OutlineNode still relies on the invisible collapse-toggle and only subtle collapsed-bullet styling, so add a visible collapse/expand affordance in the fallback path. Update OutlineNode’s non-virtualized rendering to mirror the windowed path’s rail-based controls using the existing node state and props such as hasChildren, effectiveCollapsed, data-depth, and commands.onToggleCollapsed so the toggle is clearly discoverable when virtualized is off.
🤖 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 `@src/components/OutlineRow.tsx`:
- Around line 618-697: `RailToggles` and `CollapsedRailToggle` both contain the
same `setRailHover` hover-highlighting logic, so extract that shared behavior
into a helper (for example, `highlightRailForOwner` or a small hook) that
handles the `.outline-list` lookup, one-time `mouseleave` cleanup,
`--hovered-rail-x` updates, and `data-rail-owners` scoping. Then update both
components’ `onMouseEnter` handlers to call the shared helper with the event,
`ownerId`, and `globalX`, keeping the existing hover behavior identical while
removing duplication.
- Around line 51-52: The widened rail hit area in OutlineRow is overlapping the
collapse-toggle touch target, so gutter clicks can be captured by the parent row
instead of the current row. Adjust the rail hitbox logic around RAIL_CENTER_X_PX
and RAIL_HIT_WIDTH_PX, and/or lower the rail’s effective hit area or z-index so
it no longer covers the collapse-toggle area; verify the .rail-toggle layering
still allows the intended row-specific collapse action to receive clicks.
---
Outside diff comments:
In `@src/components/OutlineNode.tsx`:
- Around line 293-304: The non-virtualized row in OutlineNode still relies on
the invisible collapse-toggle and only subtle collapsed-bullet styling, so add a
visible collapse/expand affordance in the fallback path. Update OutlineNode’s
non-virtualized rendering to mirror the windowed path’s rail-based controls
using the existing node state and props such as hasChildren, effectiveCollapsed,
data-depth, and commands.onToggleCollapsed so the toggle is clearly discoverable
when virtualized is off.
In `@src/routes/__root.tsx`:
- Around line 85-91: The AuthGate bypass currently relies only on
VITE_BYPASS_AUTH, which can be baked into shipped client builds; update AuthGate
to allow the bypass only when import.meta.env.DEV (or an equivalent explicit
non-production check) is also true, so the session gate cannot be skipped in
staging/prod. Keep the change localized to the AuthGate logic and preserve the
existing useSession, isPending, and AuthScreen flow for normal authenticated
access.
🪄 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: ba8793b5-aaf3-48e1-a547-dba597abe3a1
📒 Files selected for processing (9)
e2e/collapse-flash.spec.tse2e/mirrors.spec.tssrc/components/OutlineEditor.tsxsrc/components/OutlineNode.tsxsrc/components/OutlineRow.tsxsrc/data/visible-order.test.tssrc/data/visible-order.tssrc/routes/__root.tsxsrc/styles.css
Comment on lines
+51
to
+52
| const RAIL_CENTER_X_PX = 14; | ||
| const RAIL_HIT_WIDTH_PX = 20; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "\.collapse-toggle\s*{" -A15 src/styles.css
rg -n "\.rail-toggle\s*{" -A15 src/styles.cssRepository: cameronapak/dotflowy
Length of output: 362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'src/components/OutlineRow.tsx' 'src/styles.css'
wc -l src/components/OutlineRow.tsx src/styles.css
sed -n '1,220p' src/components/OutlineRow.tsx
printf '\n--- styles ---\n'
rg -n '\.collapse-toggle\s*\{' -A20 src/styles.css
printf '\n---\n'
rg -n '\.rail-toggle\s*\{' -A20 src/styles.cssRepository: cameronapak/dotflowy
Length of output: 10407
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '220,460p' src/components/OutlineRow.tsx
printf '\n---\n'
sed -n '460,720p' src/components/OutlineRow.tsxRepository: cameronapak/dotflowy
Length of output: 18125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\.outline-row\s*\{|\.outline-node\s*\{|\.touch-hitbox\s*\{|\.collapse-toggle|\.rail-toggle|\.collapsed-rail-toggle' -A18 src/styles.css
printf '\n---\n'
sed -n '720,860p' src/components/OutlineRow.tsxRepository: cameronapak/dotflowy
Length of output: 6436
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\.touch-hitbox\b|::before' -A24 src/styles.cssRepository: cameronapak/dotflowy
Length of output: 5011
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\.touch-hitbox\b' -A20 src/styles.cssRepository: cameronapak/dotflowy
Length of output: 912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'touch-hitbox' -S .Repository: cameronapak/dotflowy
Length of output: 902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1060,1095p' src/styles.cssRepository: cameronapak/dotflowy
Length of output: 1475
RAIL_HIT_WIDTH_PX overlaps the row collapse target
The widened rail hitbox reaches into the collapse-toggle’s touch area, and .rail-toggle sits above it (z-index: 3), so clicks near the gutter can trigger the parent collapse action instead of the current row’s collapse.
🤖 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 `@src/components/OutlineRow.tsx` around lines 51 - 52, The widened rail hit
area in OutlineRow is overlapping the collapse-toggle touch target, so gutter
clicks can be captured by the parent row instead of the current row. Adjust the
rail hitbox logic around RAIL_CENTER_X_PX and RAIL_HIT_WIDTH_PX, and/or lower
the rail’s effective hit area or z-index so it no longer covers the
collapse-toggle area; verify the .rail-toggle layering still allows the intended
row-specific collapse action to receive clicks.
Comment on lines
+618
to
+697
| function RailToggles({ | ||
| depth, | ||
| ownerIds, | ||
| onCollapse, | ||
| }: { | ||
| depth: number; | ||
| ownerIds: string[]; | ||
| onCollapse: (ownerId: string) => void; | ||
| }) { | ||
| if (ownerIds.length === 0) return null; | ||
|
|
||
| const setRailHover = (e: React.MouseEvent, ownerId: string, globalX: number) => { | ||
| const list = (e.currentTarget as HTMLElement).closest<HTMLElement>(".outline-list"); | ||
| if (!list) return; | ||
|
|
||
| const clearAll = () => { | ||
| list.style.removeProperty("--hovered-rail-x"); | ||
| list.querySelectorAll<HTMLElement>(".outline-node").forEach((li) => | ||
| li.classList.remove("rail-hovered") | ||
| ); | ||
| delete list.dataset.railHoverListener; | ||
| }; | ||
|
|
||
| // Attach a one-time clearer when the pointer leaves the whole list area. | ||
| // This keeps the highlight stable while the mouse travels between | ||
| // vertically adjacent segments of the same rail. | ||
| if (!list.dataset.railHoverListener) { | ||
| list.addEventListener("mouseleave", clearAll, { once: true }); | ||
| list.dataset.railHoverListener = "1"; | ||
| } | ||
|
|
||
| list.style.setProperty("--hovered-rail-x", `${globalX}px`); | ||
|
|
||
| // Scope the highlight to only the "collapsable part": rows whose ancestry | ||
| // includes this specific ownerId (i.e. descendants under that ancestor). | ||
| // This avoids lighting the entire depth column across sibling branches. | ||
| list.querySelectorAll<HTMLElement>(".outline-node").forEach((li) => { | ||
| const rowOwners = (li.dataset.railOwners || "").split(",").filter(Boolean); | ||
| if (rowOwners.includes(ownerId)) { | ||
| li.classList.add("rail-hovered"); | ||
| } else { | ||
| li.classList.remove("rail-hovered"); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| {ownerIds.map((ownerId, railIndex) => ( | ||
| <button | ||
| key={`${railIndex}:${ownerId}`} | ||
| type="button" | ||
| className="rail-toggle" | ||
| data-rail-owner-id={ownerId} | ||
| aria-label="Collapse branch" | ||
| tabIndex={-1} | ||
| style={{ | ||
| left: | ||
| RAIL_CENTER_X_PX + | ||
| railIndex * INDENT_PX - | ||
| depth * INDENT_PX - | ||
| RAIL_HIT_WIDTH_PX / 2, | ||
| }} | ||
| onMouseDown={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| }} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onCollapse(ownerId); | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| const globalX = RAIL_CENTER_X_PX + railIndex * INDENT_PX; | ||
| setRailHover(e, ownerId, globalX); | ||
| }} | ||
| /> | ||
| ))} | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract the duplicated setRailHover hover logic.
RailToggles and CollapsedRailToggle each implement a nearly identical setRailHover closure (list lookup, one-time mouseleave cleanup, --hovered-rail-x update, and data-rail-owners scoping). Extracting a shared helper (e.g., useRailHover or a module-level function taking the event/ownerId/globalX) would remove ~30 lines of duplicated logic and avoid future drift between the two copies.
♻️ Sketch of a shared helper
+function highlightRailForOwner(e: React.MouseEvent, ownerId: string, globalX: number) {
+ const list = (e.currentTarget as HTMLElement).closest<HTMLElement>(".outline-list");
+ if (!list) return;
+ const clearAll = () => {
+ list.style.removeProperty("--hovered-rail-x");
+ list.querySelectorAll<HTMLElement>(".outline-node").forEach((li) =>
+ li.classList.remove("rail-hovered")
+ );
+ delete list.dataset.railHoverListener;
+ };
+ if (!list.dataset.railHoverListener) {
+ list.addEventListener("mouseleave", clearAll, { once: true });
+ list.dataset.railHoverListener = "1";
+ }
+ list.style.setProperty("--hovered-rail-x", `${globalX}px`);
+ list.querySelectorAll<HTMLElement>(".outline-node").forEach((li) => {
+ const owners = (li.dataset.railOwners || "").split(",").filter(Boolean);
+ li.classList.toggle("rail-hovered", owners.includes(ownerId));
+ });
+}Both RailToggles and CollapsedRailToggle would then call highlightRailForOwner(e, ownerId, globalX) in their onMouseEnter.
Also applies to: 699-762
🤖 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 `@src/components/OutlineRow.tsx` around lines 618 - 697, `RailToggles` and
`CollapsedRailToggle` both contain the same `setRailHover` hover-highlighting
logic, so extract that shared behavior into a helper (for example,
`highlightRailForOwner` or a small hook) that handles the `.outline-list`
lookup, one-time `mouseleave` cleanup, `--hovered-rail-x` updates, and
`data-rail-owners` scoping. Then update both components’ `onMouseEnter` handlers
to call the shared helper with the event, `ownerId`, and `globalX`, keeping the
existing hover behavior identical while removing duplication.
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.
What
Two tree-helper improvements:
Scoped hover highlight - Previously, hovering a guide rail highlighted the entire depth column (all branches at that indent level). Now scoped using the specific ancestor ownerId from railOwnerIds, so only the segments under the hovered collapsible ancestor light up. Normal guides stay flat.
Wider clickable rails - RAIL_HIT_WIDTH_PX 12px to 20px, making the vertical guide lines much easier to click for collapse/expand without changing visuals.
Rail centering fix - 1px guide lines now center precisely on bullet-dot axes (shifted 0.5px to account for sub-pixel rendering).
Screenshots
Base outline with tree guides:
Hovering "Website redesign" rail — only its 3 children highlight:
Hovering "Projects" rail — all 7 descendants highlight:
Summary by CodeRabbit
New Features
Bug Fixes