Skip to content

Keyboard navigation for rowBasedSelection mode#119

Open
ckosmowski wants to merge 19 commits into
EvHaus:masterfrom
ckosmowski:feature-rowkeyboard
Open

Keyboard navigation for rowBasedSelection mode#119
ckosmowski wants to merge 19 commits into
EvHaus:masterfrom
ckosmowski:feature-rowkeyboard

Conversation

@ckosmowski
Copy link
Copy Markdown
Collaborator

Added Keyboard navigation for rowBasedSelection mode. (Using keyboard navigation and selection in a way that makes more sense when used with rowBasedSelection)

In row based mode: Added support for keyboard navigation which makes more sense in the row based mode.

1) Introduced seperated "handleKeyDownRowBased" method which handles key events when in row based mode
2) Added method "handleNavigateKey" as a shortcut for keys which need to commit cell edit first
3) Added option "selectOnNavigate" which enables the user to select rows via the keyboard when in row based mode

Code duplication in handleKeyDown and handleKeyDownRowBased can possibly be resolved further. Kept the code in seperate methods for now to be sure to preserve the default behaviour.
@EvHaus EvHaus added this to the v0.0.8 milestone Aug 21, 2014
@EvHaus EvHaus self-assigned this Aug 21, 2014
Comment thread src/doby-grid.js
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah, there is a lot of code duplication here. I recommend having 'if self.options.rowBasedSelection' statements inside the main handleKeyDown method instead where needed. That way you can also have a little comment explaining why the keyboard navigation does something different for rowBasedSelection (ie. "When using rowBasedSelection, up/down keys change the selection of rows instead of moving the active cell.)

@EvHaus EvHaus modified the milestone: v0.1.0 Dec 16, 2014
@ckosmowski
Copy link
Copy Markdown
Collaborator Author

Hope it's still mergeable at all.

@EvHaus EvHaus added ready and removed in progress labels Dec 19, 2014
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.

2 participants