-
Notifications
You must be signed in to change notification settings - Fork 74
Table : update edit mode tab to navigate #2870
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for moduswebcomponents ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@prashanthr6383 Regarding the issue Apart from the issue you brought up, I see the autocomplete dropdown and date picker dropdown is not appearing as before when I press tab. Can you check that? |
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.
File: stencil-workspace/src/components/modus-table/parts/cell/modus-table-cell-main/modus-table-cell-main.tsx (line 174-186)
The tab navigation code in this block could be refactored into a separate method to reduce duplication with the existing NavigateTableCells utility. The tab and shift+tab handling logic in this component is complex and should be better documented.
File: stencil-workspace/src/components/modus-table/utilities/table-cell-navigation.utility.tsx (line 33-45)
The navigation utility now handles 'shift+tab' and 'tab' cases, but uses different logic than arrow key navigation. Consider unifying the approach for better maintainability. Also, the naming and type safety could be improved - prevCell and nextCell are both of type HTMLElement, but later in the code they're accessed as if they have specific Stencil component methods.
File: stencil-workspace/src/components/modus-table/parts/cell/modus-table-cell-main/modus-table-cell-main.tsx (line 187)
The call to componentOnReady().then() assumes the component will always be ready and doesn't handle potential errors or rejection. Consider adding error handling for cases where the component might not be ready.
File: stencil-workspace/src/components/modus-table/utilities/table-cell-navigation.utility.tsx (line 17-18)
The new optional parameters (isCellEditable and onNavigateComplete) are only used for tab navigation. Consider separating these concerns or add documentation explaining why these parameters are tied specifically to tab navigation.
Description
References
Fixes #2437
Type of change
How Has This Been Tested?
Checklist