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

Add option to keep caret inside block in edit mode #23546

Merged
merged 3 commits into from
Jul 1, 2020

Conversation

tellthemachines
Copy link
Contributor

@tellthemachines tellthemachines commented Jun 29, 2020

Description

Fixes #22190, builds on #22204.

Added "Keyboard options" section in "More tools & options > Options" modal, with checkbox labelled "Keep cursor inside active block". Checking it will stop arrow keys from navigating between blocks in edit mode. Navigation mode is not affected.

How has this been tested?

Tested in browser with keyboard.

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@tellthemachines tellthemachines added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [a11y] Keyboard & Focus labels Jun 29, 2020
@tellthemachines tellthemachines self-assigned this Jun 29, 2020
@github-actions
Copy link

github-actions bot commented Jun 29, 2020

Size Change: -338 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/a11y/index.js 1.14 kB +1 B
build/api-fetch/index.js 3.4 kB -1 B
build/autop/index.js 2.82 kB +1 B
build/block-editor/index.js 109 kB +73 B (0%)
build/block-editor/style-rtl.css 10.7 kB -2 B (0%)
build/block-editor/style.css 10.7 kB -3 B (0%)
build/block-library/editor-rtl.css 7.41 kB -186 B (2%)
build/block-library/editor.css 7.41 kB -187 B (2%)
build/block-library/index.js 129 kB -189 B (0%)
build/block-library/style-rtl.css 8.04 kB +5 B (0%)
build/block-library/style.css 8.05 kB +5 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB +1 B
build/blocks/index.js 48.2 kB +3 B (0%)
build/components/index.js 198 kB -23 B (0%)
build/components/style-rtl.css 15.9 kB +6 B (0%)
build/components/style.css 15.9 kB +5 B (0%)
build/compose/index.js 9.65 kB -1 B
build/core-data/index.js 11.4 kB -1 B
build/data-controls/index.js 1.29 kB +1 B
build/data/index.js 8.44 kB +6 B (0%)
build/date/index.js 5.47 kB +3 B (0%)
build/edit-navigation/index.js 9.88 kB +16 B (0%)
build/edit-post/index.js 304 kB +68 B (0%)
build/edit-post/style-rtl.css 5.51 kB +5 B (0%)
build/edit-post/style.css 5.51 kB +5 B (0%)
build/edit-site/index.js 16.6 kB -10 B (0%)
build/edit-site/style-rtl.css 3 kB -34 B (1%)
build/edit-site/style.css 3 kB -36 B (1%)
build/edit-widgets/index.js 9.32 kB -4 B (0%)
build/editor/index.js 44.9 kB +102 B (0%)
build/editor/style-rtl.css 3.85 kB +9 B (0%)
build/editor/style.css 3.86 kB +10 B (0%)
build/element/index.js 4.65 kB -1 B
build/i18n/index.js 3.56 kB -1 B
build/is-shallow-equal/index.js 710 B -1 B
build/keyboard-shortcuts/index.js 2.52 kB +5 B (0%)
build/keycodes/index.js 1.94 kB +1 B
build/list-reusable-blocks/index.js 3.12 kB -1 B
build/notices/index.js 1.79 kB -2 B (0%)
build/nux/index.js 3.4 kB -1 B
build/nux/style-rtl.css 671 B +8 B (1%)
build/nux/style.css 668 B +8 B (1%)
build/plugins/index.js 2.56 kB -1 B
build/rich-text/index.js 14 kB +1 B
build/server-side-render/index.js 2.67 kB -2 B (0%)
build/shortcode/index.js 1.69 kB -1 B
build/token-list/index.js 1.28 kB +1 B
build/wordcount/index.js 1.17 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/annotations/index.js 3.62 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.39 kB 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B

compressed-size-action

@enriquesanchez
Copy link
Contributor

Hi @tellthemachines 👋

I just tested this PR and everything worked as expected to me.

Screen Shot 2020-06-29 at 17 52 44

I saw the new option and after turning it on, I was able to confirm that the caret stays constrained in the block. Turning it back off took me back to the default behaviour.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Code wise this is looking good.

cc @mtias @jasmussen do you have thoughts here?

@jasmussen
Copy link
Contributor

This seems good.

On the one hand, it's good that the label is short and simple. But on the other hand, this is quite a profound change to the editor — it may be exactly what you're looking for, or it may break your experience entirely. I wonder if there's a more descriptive label we can find, or whether this may even be a place to add help text. "Cursor", for example, may even be imprecise, as you can move your cursor anywhere.

Short option:

[ ] Contain text cursor inside active block

This is a bit more technical, actually, so not sure it's an improvement. But it feels more precise.

Longer option:

[ ] Contain text cursor inside active block
_This option can benefit users of assistive technologies._

Regardless of what we go with, I would say it's good to merge sooner rather than later, because it's a good option and we can alwasy polish the label if need be.

@alexstine
Copy link
Contributor

Tested using NV Access in Firefox, works fine.

@tellthemachines
Copy link
Contributor Author

@jasmussen I went with the short option for now; we can always review it later.

Thanks everyone for your feedback!

@tellthemachines tellthemachines merged commit 01e3b7d into master Jul 1, 2020
@tellthemachines tellthemachines deleted the add/keep-caret-inside-block branch July 1, 2020 02:23
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jul 1, 2020
@ZebulanStanphill ZebulanStanphill added the [Type] Feature New feature to highlight in changelogs. label Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Feature New feature to highlight in changelogs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edit mode: arrow up/down should not move to other blocks
6 participants