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

keyboard navigation for field selection #6969

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ehconitin
Copy link
Contributor

Adressing #6954
@Bonapara

How about this behaviour? The field wont be selected initially and then it is persistent once selected?

2024-09-10.18-16-32.mp4

Key navigation -
I couldnt show key strokes on screen like u do :)

2024-09-10.18-09-55.mp4

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request implements keyboard navigation for field type selection in the data model settings, addressing issue #6954. Key changes include:

  • Added keyboard navigation using Tab, Shift+Tab, and Enter keys in SettingsDataModelFieldTypeSelect component
  • Introduced new SettingsDataModelHotkeyScope enum for managing hotkey scopes
  • Created SettingsDataModelFieldConfigurationForm component for improved field configuration UI
  • Updated SettingsObjectNewFieldStep2 to support toggling between field type selection and configuration steps
  • Implemented styling updates for focused and active states to improve visual feedback

These changes enhance user experience and accessibility in the custom field creation process, aligning with the desired behavior outlined in the issue.

4 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings

<SettingsDataModelFieldSettingsFormCard
fieldMetadataItem={{
icon: formConfig.watch('icon'),
label: formConfig.watch('label') || 'Employees',
Copy link

Choose a reason for hiding this comment

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

style: Consider using a more generic default label instead of 'Employees'

@Bonapara
Copy link
Member

As discussed in the video call, let's deactivate the navigation in the breadcrumb step selector when no field type has been selected. Additionally, disable the Cancel and Save buttons on step one!

@ehconitin
Copy link
Contributor Author

@Bonapara
Thanks for the help, this looks much cleaner! :)

2024-09-11.13-58-00.mp4

@ehconitin
Copy link
Contributor Author

@lucasbordeau could you please review this one?

Thanks

@lucasbordeau
Copy link
Contributor

Could you please add a story so the coverage gets above 50 % ?

@ehconitin
Copy link
Contributor Author

@lucasbordeau These already have their corresponding stories!
Not sure if you are asking for something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants