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 handling for grid components in font color and list style #12202

Merged

Conversation

mmotyczynska
Copy link
Contributor

@mmotyczynska mmotyczynska commented Aug 2, 2022

Suggested merge commit message (convention)

Feature (ui): Introduced a common addKeyboardHandlingForGrid() helper to handle grid keyboard navigation. Closes #11851.

Fix (font): Fixed focus order for color grid in font color and background dropdowns. Closes #11841.

Fix (list): Unified focus handling and keyboard navigation in list properties dropdowns. Closes #11041.


Additional information

Font color / background List style
video3242862938.mp4
video5242862938.mp4

Copy link
Member

@oleq oleq left a comment

Choose a reason for hiding this comment

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

The direction is right. Refactoring needed, though.

packages/ckeditor5-ui/src/dropdown/utils.js Outdated Show resolved Hide resolved
packages/ckeditor5-ui/src/dropdown/utils.js Outdated Show resolved Hide resolved
packages/ckeditor5-ui/src/dropdown/utils.js Outdated Show resolved Hide resolved
packages/ckeditor5-table/src/ui/inserttableview.js Outdated Show resolved Hide resolved
packages/ckeditor5-table/src/ui/inserttableview.js Outdated Show resolved Hide resolved
…een styles but only between other items in dropdown).
…st styles to stylesView from the ListPropertiesView.
…vigation with arrows in one place for all color grids.
…ow left/right) will be handled in addKeyboardHandlingForGrid.
Improves also selection: mouseover and keyboard navigation share the same logic now.
We added new arrow navigation by helper function addKeyboardHandlingForGrid so the focus cycler was not needed anymore.
…der of render() and appendGrids() in unit tests.
Copy link
Member

@oleq oleq left a comment

Choose a reason for hiding this comment

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

The direction looks good. Mostly code refactoring suggestions and some questions.

packages/ckeditor5-ui/src/colorgrid/colorgridview.js Outdated Show resolved Hide resolved
packages/ckeditor5-font/tests/ui/colortableview.js Outdated Show resolved Hide resolved
@mmotyczynska mmotyczynska requested a review from oleq August 11, 2022 10:19
Copy link
Member

@oleq oleq left a comment

Choose a reason for hiding this comment

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

Just waiting for the CI.

@oleq oleq merged commit 5e8cdcb into master Aug 12, 2022
@oleq oleq deleted the ck/11851-keyboard-handling-for-grid-components-discovery-poc branch August 12, 2022 11:54
@mlewand mlewand changed the title Keyboard handling for grid components discovery PoC Keyboard handling for grid components in font color and list style Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants