dbeaver/pro#8402 feat: enable checkbox selection via keyboard in Team…#4445
dbeaver/pro#8402 feat: enable checkbox selection via keyboard in Team…#4445SychevAndrey wants to merge 2 commits into
Conversation
…s and Users tables
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| function handleHeaderKeyDown(event: DataGridCellKeyboardEvent) { | ||
| if (!selection || selection.keys.length === 0 || (event.key !== 'Enter' && event.key !== ' ')) { | ||
| return; | ||
| } | ||
|
|
||
| const colIndex = event.currentTarget.closest('[aria-colindex]')?.getAttribute('aria-colindex'); | ||
|
|
||
| if (colIndex !== SELECT_COLUMN_ARIA_INDEX) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| selection.selectRoot(); | ||
| } |
There was a problem hiding this comment.
seems like we can make a common function for all usages among the components. and reuse it
|
|
||
| return ( | ||
| <Checkbox disabled={disabled || selection.keys.length === 0} checked={checked} indeterminate={indeterminate} onChange={selection.selectRoot} /> | ||
| <div className={CELL_CLASS_NAME} onClick={rootDisabled ? undefined : selection.selectRoot}> |
There was a problem hiding this comment.
why is not Command too?
There was a problem hiding this comment.
Please use checkbox with indeterminate state. Please do not break semantics of native checkbox element by changing it to the div. We need to find another way to fix initial issue
| const checked = selection.selected.includes(id); | ||
|
|
||
| return ( | ||
| <Command className={CELL_CLASS_NAME} disabled={disabled} tabIndex={0} onClick={() => selection.select(id)}> |
There was a problem hiding this comment.
why is it Command + Indicator implementation? not like a Checkbox itself?
Checkbox supposed to be tabable and clickable via keyboard
There was a problem hiding this comment.
Yes, it should be checkbox, not command
| const checked = selection.selected.includes(id); | ||
|
|
||
| return ( | ||
| <Command className={CELL_CLASS_NAME} disabled={disabled} tabIndex={0} onClick={() => selection.select(id)}> |
There was a problem hiding this comment.
Yes, it should be checkbox, not command
|
|
||
| return ( | ||
| <Checkbox disabled={disabled || selection.keys.length === 0} checked={checked} indeterminate={indeterminate} onChange={selection.selectRoot} /> | ||
| <div className={CELL_CLASS_NAME} onClick={rootDisabled ? undefined : selection.selectRoot}> |
There was a problem hiding this comment.
Please use checkbox with indeterminate state. Please do not break semantics of native checkbox element by changing it to the div. We need to find another way to fix initial issue
…rove accessibility in tables fix checkbox and header cell keyboard events
3 participants
…s and Users tables