🎨 Palette: Standard-Compliant Focus Restoration for Overlays - #189
Conversation
- Captures document.activeElement upon opening Server Details Panel or Connection Credentials Modal. - Restores focus to the triggering element when the overlay is closed, dismissed, or saved. - Prevents keyboard users and screen readers from losing focus state and resetting back to the top of the document. - Fully tested using NodeJS DOM simulation script in pytest. Co-authored-by: SoulWayy <285978010+SoulWayy@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe dashboard captures the active element when opening the detail panel or credentials modal. It restores focus after closing when the element remains connected and focusable. Refreshes preserve the stored invoker. Tests cover these behaviors. ChangesOverlay focus restoration
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Trigger
participant Dashboard
participant Overlay
participant RefreshSource
Trigger->>Dashboard: Open detail panel or credentials modal
Dashboard->>Overlay: Store trigger and show overlay
RefreshSource->>Dashboard: Refresh open detail view
Dashboard->>Overlay: Preserve stored trigger
Trigger->>Dashboard: Close overlay
Dashboard->>Trigger: Restore focus when connected
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.Jules/palette.md:
- Around line 32-34: Vertaal de leerregel onder “2026-07-24 -
[Standard-Compliant Focus Restoration for Overlays]” naar het Nederlands en voeg
een lege regel direct na de heading toe. Behoud de bestaande inhoud en
structuur, inclusief de secties “Learning” en “Action”, maar formuleer alle
tekst in het Nederlands.
In `@tests/test_dashboard.py`:
- Around line 484-564: Breid _FOCUS_HARNESS en
test_focus_restoration_behavior_node uit met uitvoerbare dekking voor
openCredentialsModal en closeCredentialsModal. Gebruik afzonderlijke focusbare
invokers en verifieer dat credInvoker bij openen wordt vastgelegd, bij sluiten
wordt gewist en focus alleen wordt hersteld voor verbonden elementen, inclusief
een BODY-invoker; dek ook een niet-verbonden invoker af volgens het bestaande
DOM-gedrag.
- Around line 525-533: Remove the trivial inline comments “Test openDetail” and
“Test closeDetail” from the JavaScript harness, leaving the surrounding
openDetail, closeDetail, and assertion-related statements unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 95b0ba14-63e9-4f7d-9cdb-da358116c923
📒 Files selected for processing (4)
.Jules/palette.md.agents/eval/scorecard.jsonsrc/kater/web/dashboard.pytests/test_dashboard.py
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
- Captures document.activeElement upon opening Server Details Panel or Connection Credentials Modal. - Restores focus to the triggering element when the overlay is closed, dismissed, or saved. - Prevents keyboard users and screen readers from losing focus state and resetting back to the top of the document. - Fully tested using NodeJS DOM simulation script in pytest. Co-authored-by: SoulWayy <285978010+SoulWayy@users.noreply.github.com>
Re-apply detail-panel invoker retargeting, credential-modal Node harness coverage, and Dutch palette learning text that tip 56ecd1d undid. Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/kater/web/dashboard.py`:
- Around line 2373-2376: Prevent handleWSMessage status refreshes from
overwriting detailInvoker while the detail panel is open. Update
openServerDetail/openDetail so explicit server-selection paths pass the selected
row or card before asynchronous fetching, and only those paths set the invoker;
leave detailInvoker unchanged during background refreshes. Add a Node test
covering an open detail view with focus outside the panel during a WebSocket
refresh, verifying closeDetail restores focus to the last selected server row.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9f393ee6-accf-4be3-9815-7cac1c1d7948
📒 Files selected for processing (3)
.Jules/palette.mdsrc/kater/web/dashboard.pytests/test_dashboard.py
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_dashboard.py (1)
563-588: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest de refresh-contracten via echte callers.
Deze test voert alleen
openDetail()encloseDetail()uit. Een regressie inopenServerDetail()dierefreshniet doorgeeft, of in de succesvollesaveCredentials()-refresh, blijft daardoor ongedetecteerd. Voeg een Node-case toe met een stub voorapi()en test minstensopenServerDetail()en de credential-save-flow.🤖 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 `@tests/test_dashboard.py` around lines 563 - 588, Breid test_focus_restoration_behavior_node uit met een Node-case die api() stubt en de echte callers openServerDetail() en de succesvolle saveCredentials()-flow uitvoert. Controleer daarbij dat beide refresh-aanroepen de verwachte refresh-parameter doorgeven, zodat regressies in deze callers naast openDetail() en closeDetail() worden gedetecteerd.
🤖 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/kater/web/dashboard.py`:
- Around line 2376-2381: Capture the selected row/card before the asynchronous
detail fetch and pass that original invoker into openDetail(), rather than
reading document.activeElement afterward; update src/kater/web/dashboard.py
lines 2376-2381. Capture the credential invoker before promptCredentials() and
pass it into openCredentialsModal() at src/kater/web/dashboard.py lines
2501-2505. Update tests/test_dashboard.py lines 540-558 to use a delayed
response, move focus during the fetch, and assert that the original invoker is
retained.
- Around line 3575-3578: Update openServerDetail to track request generations or
cancel previous requests, and apply the fetched data only when the request is
still current. For refresh=true, additionally require the detail panel to remain
open and selectedNode.name to still match name before calling openDetail(data,
refresh).
In `@tests/test_dashboard.py`:
- Around line 540-558: Maak de regressietest rond openDetail asynchroon met een
deferred promise die de API-fetchrespons controleert. Laat openDetail starten,
verplaats de focus naar commandBar vóór het vrijgeven van de deferred response,
wacht daarna op afronding en controleer dat detailInvoker de oorspronkelijk
geselecteerde rij cardB blijft; behoud de bestaande close- en focusasserties.
---
Outside diff comments:
In `@tests/test_dashboard.py`:
- Around line 563-588: Breid test_focus_restoration_behavior_node uit met een
Node-case die api() stubt en de echte callers openServerDetail() en de
succesvolle saveCredentials()-flow uitvoert. Controleer daarbij dat beide
refresh-aanroepen de verwachte refresh-parameter doorgeven, zodat regressies in
deze callers naast openDetail() en closeDetail() worden gedetecteerd.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2e3c3186-3678-46bf-9902-452a8cd6528f
📒 Files selected for processing (2)
src/kater/web/dashboard.pytests/test_dashboard.py
Pass pre-fetch invokers into openDetail/openCredentialsModal, drop stale openServerDetail responses via a generation counter, and cover the async refresh contract in the Node harness so CodeRabbit findings on tip 40a6658 stay closed. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
💡 What:
Implemented standard-compliant keyboard focus restoration for the dashboard's sliding panels and modal dialogs. Specifically:
document.activeElementasdetailInvokerwhenopenDetail()is called.document.activeElementascredInvokerwhenopenCredentialsModal()is called.closeDetail()andcloseCredentialsModal()(as well as when saving credentials succeeds).🎯 Why:
Closing modals or sliding panels without focus restoration disrupts keyboard navigation by forcing screen readers and keyboard users to restart navigation from the top of the HTML document. Restoring focus preserves the active context and keeps navigation extremely natural.
📸 Before/After:
♿ Accessibility:
Fully implements WCAG 2.1 focus restoration guidelines (Standard WAI-ARIA Dialog focus management) to support keyboard-only operators and screen reader users.
PR created automatically by Jules for task 13136720007452404243 started by WaySoul (@SoulWayy)
Greptile Summary
This change adds focus restoration for the dashboard detail panel and credentials modal. Two keyboard flows remain broken in
src/kater/web/dashboard.py: saving credentials restores focus to Connect and then removes that control during refresh, and switching servers in an open detail panel returns focus to the originally opened server instead of the current server. Both failures were reproduced in Chromium.Confidence Score: 4/5
What T-Rex did
Comments Outside Diff (2)
src/kater/web/dashboard.py, line 2572-2579 (link)A successful credential save calls
closeCredentialsModal()before refreshing the catalog and reopening the detail panel. The modal briefly restores focus to the Connect trigger, but the refreshed configured state hides that trigger, leaving browser focus onBODY. Restore focus after the refreshed state is known, targeting a stable visible detail-panel element or action.Artifacts
PR-189 live browser reproduction source
PR-189 live browser reproduction observed output
Evidence from the check
Command output from the check
▶ Recording of the check
Credential modal before the successful save screenshot
▶ Recording of the check
Dashboard after successful credential save and refresh screenshot
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
General comment
openDetail()assignsdetailInvokeronly when it is null (lines 2370–2373), so selecting B does not replace A.closeDetail()restores the retained A element (lines 2459–2464).detailInvokerwhen a different server is selected while the detail panel is open, or explicitly maintain the intended return target as the currently selected server row.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "🎨 Palette: Standard-Compliant Focus Res..." | Re-trigger Greptile