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

fix navigation conflict with SelectRange and Edit #4516

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

azmy60
Copy link
Collaborator

@azmy60 azmy60 commented Jun 11, 2024

Using Up/Down Arrow keys while editing a cell can move the editor focus which is an uncommon behavior (comparing to Google Sheets and Ms. Excel). And it looks like we have 2 modules that can control table navigation => SelectRange module and Edit module. I think it's best if we can let only one module to control navigation.

Tabulator

tabulator-nav

Expected: Pressing arrow keys while editing should move the cursor inside the editor instead of continuing to navigate the table.

Google Sheets

gsheet-nav

@olifolkerd
Copy link
Owner

Hey @azmy60

Thanks for the PR. Got a few questions for you here.

First off, do you think you could manage this via confirm events, rather than direct module interaction, as when i merged in the spreadsheet module to start with, the idea is to decouple the modules rather than have them directly manipulate eachother. (for example other modules in future may also want to control this behaviour, so a general event is more useful.

Second, several of the editors control this behaviour directly. for example the textarea editor that allows the setting of the verticalNavigation property to control if arrow keys navigate or edit. How does this change interact with that? https://tabulator.info/docs/6.2/edit#editor-textarea

Cheers

Oli :)

@olifolkerd
Copy link
Owner

Hey @azmy60

Just wondering what your thoughts were on this one?

Cheers

Oli :)

@azmy60
Copy link
Collaborator Author

azmy60 commented Oct 3, 2024

Thanks for the feedback! I didn't know the verticalNavigation exists so I'll check that and also the confirm events sound very good! I'll try putting everything together next week.

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