-
Notifications
You must be signed in to change notification settings - Fork 13
Keyboard navigation #283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keyboard navigation #283
Changes from all commits
fa6253f
36122c4
0da582d
1b9fc8f
0212dc2
52da893
02c1743
23c4e0f
fec6e2d
ebaa449
36870cb
b912304
b92a36f
c062bf5
c3a4021
aa7e6c9
b0b761f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { ReactNode } from 'react' | ||
|
|
||
| export default function Focusable({ | ||
| ariaLabel, | ||
| children, | ||
| inactive, | ||
| onEscape, | ||
| onEnter, | ||
| round, | ||
| role, | ||
| fit, | ||
| id, | ||
| }: { | ||
| ariaLabel?: string | ||
| children: ReactNode | ||
| inactive?: boolean | ||
| onEscape?: () => void | ||
| onEnter?: () => void | ||
| round?: boolean | ||
| role?: string | ||
| fit?: boolean | ||
| id?: string | ||
| }) { | ||
| const style: React.CSSProperties = { | ||
| borderRadius: round ? '0.5rem' : undefined, | ||
| width: fit ? 'fit-content' : '100%', | ||
| } | ||
|
|
||
| const handleKeyDown = (e: React.KeyboardEvent) => { | ||
| if (onEnter && ['Enter', ' '].includes(e.key)) { | ||
| e.stopPropagation() | ||
| e.preventDefault() | ||
| onEnter() | ||
| } | ||
| if (onEscape && e.key === 'Escape') { | ||
| e.stopPropagation() | ||
| e.preventDefault() | ||
| onEscape() | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <div | ||
| id={id} | ||
| style={style} | ||
| className='focusable' | ||
| role={role ?? 'button'} | ||
| onKeyDown={handleKeyDown} | ||
| tabIndex={inactive ? -1 : 0} | ||
| aria-label={ariaLabel ?? 'Focusable element'} | ||
| > | ||
| {children} | ||
| </div> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,15 +15,12 @@ interface InputPasswordProps { | |
| export default function InputPassword({ focus, label, onChange, onEnter, strength, placeholder }: InputPasswordProps) { | ||
| const right = strength ? <StrengthLabel strength={strength} /> : undefined | ||
|
|
||
| const firstRun = useRef(true) | ||
| const input = useRef<HTMLIonInputElement>(null) | ||
|
|
||
| // focus input when focus prop changes | ||
| useEffect(() => { | ||
| if (focus && firstRun.current) { | ||
| firstRun.current = false | ||
| input.current?.setFocus() | ||
| } | ||
| }) | ||
| if (focus && input.current) input.current.setFocus() | ||
| }, [focus, input.current]) | ||
|
Comment on lines
+20
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Remove Including 🔎 Proposed fix // focus input when focus prop changes
useEffect(() => {
if (focus && input.current) input.current.setFocus()
- }, [focus, input.current])
+ }, [focus])🤖 Prompt for AI Agents |
||
|
|
||
| return ( | ||
| <InputContainer label={label} right={right}> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Global keyboard listener may cause conflicts.
The window-level keyboard listener for ArrowDown/ArrowUp will be active whenever this component is mounted, regardless of whether the component is focused or visible. This could interfere with other parts of the application that use these keys for navigation.
Consider one of these approaches:
🔎 Proposed fix using a container ref
🤖 Prompt for AI Agents