fix(navigation): keep active destination visible - #1608
Conversation
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe dashboard primary navigation now keeps the active destination within a 16px horizontal inset. It responds to window resizing, adds navigation padding, and includes tests for initial scrolling, visible links, resize handling, and timer cleanup. ChangesPrimary navigation scrolling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Desktop primary navigation now keeps the active destination visible with a 16 px inset during activation and resizing, while preserving mobile navigation behavior. No current merge-blocking risk is identified. Sequence Diagram(s)sequenceDiagram
participant PrimaryNavLink
participant nav
participant window
PrimaryNavLink->>nav: measure navigation and active link bounds
PrimaryNavLink->>nav: set scrollLeft when link exceeds 16px inset
window->>PrimaryNavLink: dispatch resize event
PrimaryNavLink->>nav: re-measure and update scrollLeft
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Visual inspection receipt for exact head
|
seonghobae
added
bug
ChatGPT Codex Connector
Design assurance exact-head acceptance —
|
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Follow-up repair on exact head |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact head: Root-cause repair:
Verification:
Remaining acceptance scope:
|
|
Visual Inspection receipt — exact head
|
|
Current-head repair receipt for
|
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target="ef1fd4ce6cd626585a65182ddda98b331682a3ef"
echo "== Checked-out commit =="
git rev-parse HEAD
git status --short
git cat-file -t "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
echo
echo "== Target diff inventory against first parent =="
base="$(git rev-parse "${target}^1")"
git diff --name-status "$base" "$target"
git diff --stat "$base" "$target"
echo
echo "== Changed-file outlines =="
ast-grep outline frontend/src/components/DashboardLayout.tsx --items all || true
ast-grep outline frontend/src/components/DashboardLayout.test.tsx --items all || true
echo
echo "== Production diff =="
git diff --no-ext-diff --unified=100 "$base" "$target" -- frontend/src/components/DashboardLayout.tsx
echo
echo "== Test diff =="
git diff --no-ext-diff --unified=100 "$base" "$target" -- frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Relevant implementation and test references =="
rg -n -C 5 --glob 'DashboardLayout.tsx' --glob 'DashboardLayout.test.tsx' \
'(scrollLeft|requestAnimationFrame|cancelAnimationFrame|resize|aria-current|scroll-px-3|px-4|focus|keydown|pointer|touch|ResizeObserver)' \
frontend/src/componentsLength of output: 43270 🏁 Script executed: #!/bin/bash
set -euo pipefail
target="ef1fd4ce6cd626585a65182ddda98b331682a3ef"
repo="ContextualWisdomLab/naruon"
echo "== Pull request metadata =="
gh pr view 1608 --repo "$repo" --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,url
echo
echo "== Base-to-target effective inventory =="
base="$(gh pr view 1608 --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
echo "base=$base"
git cat-file -e "${base}^{commit}"
git diff --name-status "$base" "$target"
git diff --stat "$base" "$target"
echo
echo "== DashboardLayout implementation =="
sed -n '205,275p' frontend/src/components/DashboardLayout.tsx
sed -n '350,395p' frontend/src/components/DashboardLayout.tsx
echo
echo "== Focused regression setup and assertions =="
sed -n '1,125p' frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Browser-test and configuration candidates =="
fd -HI -t f 'playwright|vitest|package\.json' . | sort
rg -n -C 4 --hidden \
'(Primary workspace navigation|aria-current="page"|getBoundingClientRect\(\)|scrollLeft|desktop.*[Ss]earch|Search.*geometry)' \
-g '*.{ts,tsx,js,mjs,cjs,json}' .Length of output: 33381 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="deebb752fb0c0095f83af79a1bb7ec33eecddef0"
target="ef1fd4ce6cd626585a65182ddda98b331682a3ef"
echo "== Exact effective diff with line context =="
git diff --no-ext-diff --unified=30 "$base" "$target" -- \
frontend/src/components/DashboardLayout.tsx \
frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Browser configuration =="
sed -n '1,240p' frontend/playwright.config.ts
sed -n '1,220p' frontend/package.json
echo
echo "== Primary-navigation browser coverage =="
sed -n '1,110p' frontend/tests/e2e/dashboard-branding.spec.ts
sed -n '330,390p' frontend/tests/e2e/dashboard-branding.spec.ts
echo
echo "== Focus-related changes in the effective delta =="
git diff --no-ext-diff --unified=10 "$base" "$target" -- \
frontend/src/components/DashboardLayout.tsx \
frontend/src/components/DashboardLayout.test.tsx \
| rg -n -C 4 '(focus|keydown|onKey|tabIndex|pointer|touch|scrollLeft|requestAnimationFrame|cancelAnimationFrame|resize|px-4)' || trueLength of output: 33614
Finding: The regression can pass while the active link remains clipped
Therefore, the test passes if a real browser clamps the write before the active destination reaches the 16px right inset. It also passes if the Add a Playwright regression at a width that overflows the desktop primary navigation. Navigate to a far-right active destination such as activeRect.left >= navRect.left + 16
activeRect.right <= navRect.right - 16This test must use the actual browser The resize lifecycle is structurally correct. Each active link owns one The delta does not change keyboard handlers or focus handling. The ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target="242bccd218a45c260334b9879256221425f2bb20"
repaired_1601="9b503911"
repo="ContextualWisdomLab/naruon"
echo "== Target and parent topology =="
git cat-file -t "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
for parent in $(git show -s --format=%P "$target"); do
echo "-- parent $parent --"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent"
if git merge-base --is-ancestor "$repaired_1601" "$parent"; then
echo "contains_repaired_1601=yes"
else
echo "contains_repaired_1601=no"
fi
echo "paths changed from parent to target:"
git diff --name-status "$parent" "$target"
done
echo
echo "== PR metadata and base-to-target inventory =="
gh pr view 1608 --repo "$repo" --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,url
base="$(gh pr view 1608 --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
echo "base=$base"
git diff --name-status "$base" "$target"
git diff --stat "$base" "$target"
echo
echo "== Child-specific two-parent comparison =="
parents=($(git show -s --format=%P "$target"))
for parent in "${parents[@]}"; do
echo "-- diff $parent..$target --"
git diff --stat "$parent" "$target"
git diff --name-status "$parent" "$target"
done
echo
echo "== Changed-file outlines =="
ast-grep outline frontend/src/components/DashboardLayout.tsx --items all
ast-grep outline frontend/src/components/DashboardLayout.test.tsx --items all
echo
echo "== Navigation implementation and component tests =="
rg -n -C 12 --glob 'DashboardLayout.tsx' --glob 'DashboardLayout.test.tsx' \
'(PrimaryNavLink|isActiveDestination|primary-navigation|scrollLeft|requestAnimationFrame|cancelAnimationFrame|addEventListener|removeEventListener|resize|px-4|aria-current|getBoundingClientRect)' \
frontend/src/components
echo
echo "== Exact target diffs for requested files against PR base =="
git diff --no-ext-diff --unified=40 "$base" "$target" -- \
frontend/src/components/DashboardLayout.tsx \
frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Browser regression candidates =="
fd -HI -t f . frontend/tests frontend 2>/dev/null | rg '(playwright|\.spec\.(ts|tsx|js)|\.e2e\.(ts|tsx|js))$' || true
rg -n -C 8 --hidden -g '*.{ts,tsx,js,mjs,cjs}' \
'(primary navigation|Primary workspace navigation|aria-current="page"|scrollLeft|getBoundingClientRect|visible inset|/settings)' \
frontend/tests frontend/src 2>/dev/null || trueLength of output: 149840
The effective child delta changes exactly these two files:
No governance or GAP file changes exist in the base-to-head diff. The other merge-parent comparison shows only the expected removal from
Finding: The regression can still pass while browser scroll clamping clips the active destination
The inspected Playwright tests do not contain a geometry assertion for the desktop primary navigation. Therefore, the regression can pass if Chromium clamps Add a Playwright regression at a constrained desktop width. Navigate to activeRect.left >= navRect.left + 16
activeRect.right <= navRect.right - 16This test must run with the real browser scroll container. It must fail if trailing space from ✅ Action performedReview finished.
|
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/naruon"
target="b4c2bb14f8e22e71173857db4f7b6a7fbfba316f"
expected_1601_prefix="25150210"
echo "== Target identity and topology =="
git cat-file -e "${target}^{commit}"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
for parent in $(git show -s --format=%P "$target"); do
git show -s --format='parent=%H%nsubject=%s' "$parent"
if git merge-base --is-ancestor "$expected_1601_prefix" "$parent" 2>/dev/null; then
echo "contains_1601_25150210=yes"
else
echo "contains_1601_25150210=no"
fi
done
echo
echo "== PR metadata and effective inventory =="
gh pr view 1608 --repo "$repo" --json number,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,url
base="$(gh pr view 1608 --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
printf 'base=%s\ntarget=%s\n' "$base" "$target"
git diff --name-status "$base" "$target"
git diff --stat "$base" "$target"
echo
echo "== Exact effective diff =="
git diff --no-ext-diff --unified=45 "$base" "$target" -- \
frontend/src/components/DashboardLayout.tsx \
frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Implementation and unit-test structure =="
ast-grep outline frontend/src/components/DashboardLayout.tsx --items all
ast-grep outline frontend/src/components/DashboardLayout.test.tsx --items all
rg -n -C 12 --glob 'DashboardLayout.tsx' --glob 'DashboardLayout.test.tsx' \
'(PrimaryNavLink|isActiveDestination|primary-navigation|aria-current|scrollLeft|requestAnimationFrame|cancelAnimationFrame|addEventListener|removeEventListener|resize|px-4|getBoundingClientRect|focus)' \
frontend/src/components
echo
echo "== Browser regression candidates and geometry assertions =="
fd -HI -t f . frontend | rg '(^|/)(tests|e2e)/.*\.(spec|test)\.(ts|tsx|js)$|playwright\.config\.(ts|js)$'
rg -n -C 10 --hidden -g '*.{ts,tsx,js,mjs,cjs}' \
'(Primary workspace navigation|primary-navigation|aria-current="page"|scrollLeft|visible inset|boundingBox\(|getBoundingClientRect\(|/settings|/search)' \
frontend/tests frontend/src 2>/dev/null || true
echo
echo "== Non-navigation effective files, if any =="
git diff --name-only "$base" "$target" | grep -vE '^frontend/src/components/DashboardLayout(\.test)?\.tsx$' || trueLength of output: 198278 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/naruon"
target="b4c2bb14f8e22e71173857db4f7b6a7fbfba316f"
base="$(gh pr view 1608 --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
echo "== PR and target =="
gh pr view 1608 --repo "$repo" --json headRefOid,baseRefOid,isDraft,url
git show -s --format='target=%H%nparents=%P%nsubject=%s' "$target"
printf 'base=%s\n' "$base"
echo
echo "== Effective child file list =="
git diff --name-status "$base" "$target"
count="$(git diff --name-only "$base" "$target" | wc -l | tr -d ' ')"
printf 'effective_file_count=%s\n' "$count"
echo
echo "== Required implementation lines at target =="
git show "$target:frontend/src/components/DashboardLayout.tsx" | nl -ba | sed -n '215,258p;378,391p'
echo
echo "== Target-only browser geometry coverage =="
git diff --no-ext-diff --unified=8 "$base" "$target" -- frontend/tests/e2e \
|| true
echo
echo "== Existing browser geometry checks for this nav =="
rg -n -U --glob '*.{spec,test}.{ts,tsx,js}' \
'Primary workspace navigation[\s\S]{0,1800}(getBoundingClientRect|boundingBox|aria-current|scrollLeft)|\
(getBoundingClientRect|boundingBox|aria-current|scrollLeft)[\s\S]{0,1800}Primary workspace navigation' \
frontend/tests/e2e || true
echo
echo "== Unit test assertions at target =="
git show "$target:frontend/src/components/DashboardLayout.test.tsx" | nl -ba | sed -n '35,118p'Length of output: 2458 🏁 Script executed: #!/bin/bash
set -euo pipefail
target="b4c2bb14f8e22e71173857db4f7b6a7fbfba316f"
echo "== PrimaryNavLink target implementation =="
git show "$target:frontend/src/components/DashboardLayout.tsx" \
| awk 'NR >= 215 && NR <= 258 { printf "%5d %s\n", NR, $0 }'
echo
echo "== Desktop primary-navigation markup =="
git show "$target:frontend/src/components/DashboardLayout.tsx" \
| awk 'NR >= 378 && NR <= 391 { printf "%5d %s\n", NR, $0 }'
echo
echo "== Relevant target unit tests =="
git show "$target:frontend/src/components/DashboardLayout.test.tsx" \
| awk 'NR >= 35 && NR <= 118 { printf "%5d %s\n", NR, $0 }'
echo
echo "== Browser-test geometry assertions for primary navigation =="
rg -n -U --glob '*.{spec,test}.{ts,tsx,js}' \
'Primary workspace navigation[\s\S]{0,1800}(getBoundingClientRect|boundingBox|aria-current|scrollLeft)|\
(getBoundingClientRect|boundingBox|aria-current|scrollLeft)[\s\S]{0,1800}Primary workspace navigation' \
frontend/tests/e2e || true
echo
echo "== Browser-test changes in effective child delta =="
base="251502101b15c328f53882d9b8e73fd3c3b7ba50"
git diff --name-status "$base" "$target" -- frontend/tests/e2eLength of output: 9862
The effective child delta modifies exactly these files:
No governance or GAP file is in the effective delta. The desktop primary navigation retains Finding: The regression still cannot detect browser scroll clamping
The inspected Playwright coverage has no geometry assertion for Add a Playwright regression at a constrained desktop width. Navigate to expect(activeRect.left).toBeGreaterThanOrEqual(navRect.left + 16);
expect(activeRect.right).toBeLessThanOrEqual(navRect.right - 16);The test must use the actual scroll container. It must fail if You are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/naruon"
target="c2acf617245a38775379ff258887b9a931baa336"
expected_parent="996e563db4457a52d0b16840e78d23f2eb2edbe4"
echo "== Target identity and topology =="
git cat-file -e "${target}^{commit}"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
for parent in $(git show -s --format=%P "$target"); do
echo "-- parent: $parent --"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent"
if git merge-base --is-ancestor "$expected_parent" "$parent"; then
echo "contains_expected_1601=yes"
else
echo "contains_expected_1601=no"
fi
echo "parent_to_target_paths:"
git diff --name-status "$parent" "$target"
done
echo
echo "== Pull request metadata and effective inventory =="
gh pr view 1608 --repo "$repo" --json number,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,url
base="$(gh pr view 1608 --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
printf 'base=%s\ntarget=%s\n' "$base" "$target"
git diff --name-status "$base" "$target"
git diff --stat "$base" "$target"
echo
echo "== Effective non-DashboardLayout paths =="
git diff --name-only "$base" "$target" \
| grep -vE '^frontend/src/components/DashboardLayout(\.test)?\.tsx$' || true
echo
echo "== Target navigation implementation and unit tests =="
ast-grep outline frontend/src/components/DashboardLayout.tsx --items all
ast-grep outline frontend/src/components/DashboardLayout.test.tsx --items all
git show "$target:frontend/src/components/DashboardLayout.tsx" \
| nl -ba | sed -n '205,275p;365,400p'
git show "$target:frontend/src/components/DashboardLayout.test.tsx" \
| nl -ba | sed -n '1,145p'
echo
echo "== Effective navigation diff =="
git diff --no-ext-diff --unified=45 "$base" "$target" -- \
frontend/src/components/DashboardLayout.tsx \
frontend/src/components/DashboardLayout.test.tsx
echo
echo "== Inherited work from expected `#1601` parent, relative to its first parent =="
parent_base="$(git rev-parse "${expected_parent}^1")"
printf 'expected_parent_base=%s\n' "$parent_base"
git diff --name-status "$parent_base" "$expected_parent"
git diff --stat "$parent_base" "$expected_parent"
echo
echo "== Relevant inherited-code references and effective changes =="
git diff --no-ext-diff --unified=20 "$parent_base" "$expected_parent" -- \
| rg -n -C 5 '(Today|today|customer|copy|security|DashboardLayout|primary-navigation|scrollLeft|resize|requestAnimationFrame)' || true
echo
echo "== Cross-references in target source =="
rg -n -C 4 --glob '*.{ts,tsx}' \
'(DashboardLayout|primary-navigation|PrimaryNavLink|scrollLeft|aria-current|Today|today|customer|security)' \
frontend/src 2>/dev/null || true
echo
echo "== Browser geometry coverage at target =="
fd -HI -t f . frontend | rg '(^|/)(tests|e2e)/.*\.(spec|test)\.(ts|tsx|js)$|playwright\.config\.(ts|js)$' || true
rg -n -U --glob '*.{spec,test}.{ts,tsx,js}' \
'Primary workspace navigation[\s\S]{0,1800}(getBoundingClientRect|boundingBox|aria-current|scrollLeft)|\
(getBoundingClientRect|boundingBox|aria-current|scrollLeft)[\s\S]{0,1800}Primary workspace navigation' \
frontend/tests/e2e 2>/dev/null || trueLength of output: 129223
The effective child delta modifies exactly these files:
No governance or GAP file re-entered the effective delta. The inherited project-folder contract changes modify The constrained-navigation repair remains intact:
Finding: The regression can still pass when browser scroll clamping clips the active destination
The inspected Playwright coverage has no desktop primary-navigation geometry assertion. Therefore, the tests can pass if Chromium clamps the requested scroll position before the active destination reaches the 16 px right inset. Add a constrained-desktop Playwright regression for a far-right route such as expect(activeRect.left).toBeGreaterThanOrEqual(navRect.left + 16);
expect(activeRect.right).toBeLessThanOrEqual(navRect.right - 16);The test must use the actual navigation scroll container. It must fail if the You are interacting with an AI system. |
Preserve the constrained-navigation two-file delta as first-parent history while adopting #1601's current parent chain, including the security regression repair and restored release-note ownership. Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@codex review Review only exact head |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Ordinary three-way integration of the three live-region changes into the active #1608 successor. Preserve the focused RED test and all existing Today/navigation source.
1 participant
Current authority — 2026-09-10
#1601@f9cbc828ce481f259c940d809ddb7fc573ea66d572bfd55426602147277b543de6103cd4b88b7644behind_by=0).Current head preserves predecessor #1608 as first parent and adopts repaired #1601 as second parent through a non-force ordinary restack. Its owned source remains the bounded active-destination visibility behavior in
DashboardLayout.tsxand the associated regression test; inherited security, Today recovery, customer-copy and skip-link source stay with ancestors.No open descendant is currently known to require another restack. Pre-restack checks/reviews are predecessor evidence only. This exact head currently has no PR-triggered hosted workflow run.
UI Delivery Gate: FAIL
Intentionality and bounded-content scope are PASS. Current-head hosted execution, rendered responsive/touch/keyboard/AT evidence, applicable locale expansion coverage and a qualifying independent review remain incomplete.