Fix keyboard navigation issues #2203
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 9c812cb in 2 minutes and 17 seconds
More details
- Looked at
224lines of code in5files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. apps/studio/src/assets/styles/app/_mixins.scss:56
- Draft comment:
Consider increasing theoutline-offsetfor:focus-visibleto ensure it is clearly visible on all displays. A value around0.2remor0.3remmight be more noticeable. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
2. apps/studio/src/components/common/modals/ConfirmationModal.vue:32
- Draft comment:
Ensure that the change in autofocus behavior in the ConfirmationModal (from "Confirm" to "Cancel" button) is intentional and documented, as it alters the user experience and expectations. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_azLz8tNPNAT7k2gk
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 3 days left in your free trial, upgrade for $20/seat/month or contact us.
| updateTextEditorFocus(focused: boolean) { | ||
| this.switchPaneFocus(undefined, focused ? 'text-editor' : 'table') | ||
| }, | ||
| switchPaneFocus(_event?: KeyboardEvent, target?: 'text-editor' | 'table') { |
There was a problem hiding this comment.
Consider refactoring the switchPaneFocus method to use a more robust and scalable approach than string comparison for managing focus targets. Using an enum or constants for focus targets could improve maintainability and reduce potential bugs.
| @@ -240,6 +245,38 @@ | |||
| } catch { | |||
| return 0; | |||
| } | |||
| }, | |||
| scrollToRangeIfOutOfView() { | |||
There was a problem hiding this comment.
Consider refactoring the scrollToRangeIfOutOfView method in the ResultTable component to avoid duplicating code. Using a shared utility function or ensuring that the Tabulator library provides this functionality natively would be more maintainable.
close #2187
Summary:
Enhances keyboard navigation and focus management across multiple components, improving accessibility and user experience.
Key points:
_mixins.scss.TabQueryEditor.vue.ConfirmationModal.vue.ResultTable.vue.TableTable.vue.Generated with ❤️ by ellipsis.dev