🎨 Palette: Enhanced Note List Accessibility and Editor UX#3
Conversation
- Add keyboard navigation (tabIndex, role="button", onKeyDown) to note list - Add :focus-visible styles for better visual feedback - Add auto-focus to title input on new note creation - Add ARIA labels to improve screen reader support - Fix unused import lint error in NoteEditor.tsx Co-authored-by: davidraehles <6085055+davidraehles@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. |
There was a problem hiding this comment.
Pull request overview
This PR improves the Secure Notes frontend’s keyboard/screen-reader accessibility for the note list and adds minor editor UX polish (auto-focus on new note), alongside a small cleanup to fix a lint issue.
Changes:
- Makes sidebar note items keyboard-focusable/selectable and adds ARIA attributes for SR support.
- Adds high-contrast
:focus-visiblestyling for note items (including the active state). - Auto-focuses the editor title field when creating a new note; removes an unused type import.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/views/NoteList.tsx | Adds keyboard handlers + ARIA labeling to custom note list items; adds delete button ARIA label. |
| frontend/src/views/NoteEditor.tsx | Adds title input auto-focus on “New Note” and ARIA labels; removes unused Note type import. |
| frontend/src/index.css | Adds :focus-visible outline styling for .note-item (and active variant). |
| .Jules/palette.md | Adds internal notes documenting the accessibility approach used. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| handleNoteClick(note); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The note item’s onKeyDown handler will also fire when the Delete is focused because keydown events bubble. This means pressing Enter/Space on the Delete button can trigger handleNoteClick(note) (and e.preventDefault may also interfere with the button’s native activation). Consider ignoring events where e.target !== e.currentTarget, or stopping propagation from the Delete button’s key handlers, so keyboard users can delete without also selecting the note.
There was a problem hiding this comment.
Added if (e.target !== e.currentTarget) return; at the start of the keydown handler to prevent Enter/Space on the Delete button from bubbling up and triggering note selection.
There was a problem hiding this comment.
Excellent point. I've added the check to ensure that only key events directly on the note item trigger selection, preventing bubbling from nested buttons.
| tabIndex={0} | ||
| role="button" | ||
| aria-label={`Note: ${note.title || 'Untitled'}`} | ||
| aria-current={currentNote?.id === note.id ? 'true' : undefined} |
There was a problem hiding this comment.
Using role="button" + tabIndex on a
There was a problem hiding this comment.
Removed role="button" from the note item div to eliminate nested interactive controls. The div retains tabIndex={0}, onClick, onKeyDown, and aria-label for keyboard and mouse accessibility without creating an invalid nested button-in-button structure.
There was a problem hiding this comment.
Good catch! Removing role="button" avoids the invalid nested interactive element structure while maintaining keyboard accessibility via tabIndex and the onKeyDown handler.
| ref={titleInputRef} | ||
| type="text" | ||
| className="note-title-input" | ||
| placeholder="Note title" | ||
| aria-label="Note title" |
There was a problem hiding this comment.
The title/content fields are now explicitly focus-managed (auto-focus on New Note), but the CSS currently removes focus outlines for .note-title-input and .note-content-input without providing an alternative. This makes keyboard focus hard to see. Consider adding a :focus-visible style (or removing outline: none) so the focused editor field is visually indicated.
There was a problem hiding this comment.
Added :focus-visible styles for both .note-title-input and .note-content-input with a 2px solid black outline. The :focus rule still removes outlines for mouse/touch users, while :focus-visible restores them for keyboard navigation.
…teractive roles, and focus visibility
- Prevent keydown event bubbling from nested buttons in NoteList - Remove invalid nested role="button" from NoteList items - Maintain keyboard accessibility via tabIndex and custom onKeyDown handler Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
This PR implements several micro-UX and accessibility improvements to the Secure Notes app:
:focus-visibleoutlines to note items to clearly show which note is currently focused.aria-labelattributes to note items, delete buttons, and editor inputs. Addedrole="button"andaria-currentto the note list.Noteimport inNoteEditor.tsxto resolve a lint error.All changes are contained within approximately 41 lines of code across three files.
PR created automatically by Jules for task 9932565411486648363 started by @davidraehles