Skip to content
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

feat: implement command menu for keyboard navigation #1259

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

siddhart1o1
Copy link
Member

@siddhart1o1 siddhart1o1 commented Jan 14, 2025

  • Open the command window with actions: cmd + k
  • Open the command window with navigation options only: Shift + n
  • Use the up and down arrows to navigate in the table component.
  • Use the right and left arrow buttons to navigate between tabs on the Customer and Credit Facility pages.

TODO

  • Open the command window with actions: cmd + k
  • Open the command window with navigation options only: Shift + n

add these instructions in app

@siddhart1o1 siddhart1o1 marked this pull request as draft January 14, 2025 18:22
Copy link

github-actions bot commented Jan 14, 2025

Storybook preview: Link to Storybook

@siddhart1o1 siddhart1o1 force-pushed the chore--command-menu-for-keyboard-nav branch from d29a9ae to 1f52ac9 Compare January 15, 2025 14:46
Copy link

github-actions bot commented Jan 15, 2025

Manuals preview: Link to Manuals

@siddhart1o1 siddhart1o1 force-pushed the chore--command-menu-for-keyboard-nav branch from b2f4329 to ace73d6 Compare January 15, 2025 17:00
@siddhart1o1 siddhart1o1 marked this pull request as ready for review January 15, 2025 17:00
@sandipndev
Copy link
Member

sandipndev commented Jan 15, 2025

The Ctrl + N opens a new tab for me, can we remove that somehow?

image

@sandipndev
Copy link
Member

Colour of the paginated table is incorrect

image

@sandipndev
Copy link
Member

Tab handling on pages is not pretty:

  • On dashboard, you need to press tab twice to go over a button
  • No way for tab to reach the breadcrumbs
  • Tab not reaching Create Button
  • Can't reach Update Telegram button with tabs (also issue on staging so not introduced by this PR)

There could be more but here are the first ones I could find

@siddhart1o1
Copy link
Member Author

siddhart1o1 commented Jan 15, 2025

The Ctrl + N opens a new tab for me, can we remove that somehow?

image

Yeah, for Windows/Linux, Alt should work. I will fix this to make it automatic based on the OS.
edit: changed to shift + N

@siddhart1o1
Copy link
Member Author

siddhart1o1 commented Jan 15, 2025

Colour of the paginated table is incorrect

image

It's not wrong. Now the table will automatically take focus if the focus is not on something else. Try creating multiple entries; it might be more obvious then. When visiting the lists pages, you can directly use the up and down arrows to navigate on rows, without using tabs to focus on them.

@siddhart1o1
Copy link
Member Author

siddhart1o1 commented Jan 15, 2025

Tab handling on pages is not pretty:

  • On dashboard, you need to press tab twice to go over a button
  • No way for tab to reach the breadcrumbs
  • Tab not reaching Create Button
  • Can't reach Update Telegram button with tabs (also issue on staging so not introduced by this PR)

There could be more but here are the first ones I could find

I have intentionally disabled tab navigation on everything except the main content on the dashboard to avoid requiring multiple tabs to reach the main content.

  • I will add tabs for the telegram update button as I think that is important.
  • I'm not sure we need tabs on the breadcrumb. With the command menu navigation system, breadcrumb tab handling should be disabled imo. Same for create btn we can use command menu for that.

Copy link
Member

@sandipndev sandipndev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@siddhart1o1 siddhart1o1 merged commit 139498e into main Jan 16, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants