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

WritingFlow breaks keyboard interaction with native inputs #48256

Open
afercia opened this issue Feb 20, 2023 · 7 comments · May be fixed by #33494
Open

WritingFlow breaks keyboard interaction with native inputs #48256

afercia opened this issue Feb 20, 2023 · 7 comments · May be fixed by #33494
Assignees
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] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Feb 20, 2023

Description

When using the Up and Down arrow keys on input fields within a block, WritingFlow may prevent the default action thus breaking the native, expected, interaction with inputs.

A while ago, this used to happen also for Left and Right arrow. That was fixed. It appears there's the need for a fix also for Up and Down arrow keys. Breaking the expected interaction for native inputs should never, ever, happen.

Step-by-step reproduction instructions

  • Go to the post editor.
  • Insert a Table block.
  • The Table block shows its placeholder with two inputs of type="number" for the number of columns and rows.
  • Tab to the Column count input.
  • Use the Up arrow key to increase the number of columns.
  • Observe the number doesn't increase.
  • Observe focus moves to the block placeholder container instead.
  • Go to Preferences > Blocks, and enable 'Contain text cursor inside block'. To my knowledge, this disables WritingFlow.
  • Repeat the steps above.
  • Observe that now the Up arrow key does increase the number of columns.

Please consider this potentially happens with any input where the Up and Down arrow keys have a native behavior, also for blocks provided by plugins or themes.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [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 Feb 20, 2023
@spencerfinnell
Copy link

I am still experiencing this with left / right arrows as well.

@afercia
Copy link
Contributor Author

afercia commented Jul 20, 2023

@spencerfinnell thanks for looking into this.

I tested the Table block again and seems to me something has changed, at least for the Table block.

In WP 6.2:

  • Left/Right arrow keys: cursor stays within the input 👍
  • Up/Down arrow keys: cursor exits input field ❌

In WP trunk, now 6.4-alpha-56267 (6.3 was branched out yesterday):

  • Left/Right arrow keys: cursor exits input field ❌
  • Up/Down arrow keys: cursor stays within the input 👍

In WP trunk, now 6.4-alpha-56267 - Gutenberg plugin latest trunk activated

  • Left/Right arrow keys: cursor exits input field ❌
  • Up/Down arrow keys: cursor stays within the input 👍

Seems to me for other blocks e.g. a Youtube video input field, there are no changes int he behavior.

The changed behavior for the Table block is a regression in trunk. Not sure what has changed, I'd greatly apprecaite some help here as it's unfortunate this regression is going to be released in WP 6.3. Cc @ellatrix @t-hamano 🙏

@afercia afercia added [Type] Regression Related to a regression in the latest release [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [a11y] Keyboard & Focus labels Jul 20, 2023
@t-hamano
Copy link
Contributor

Thanks for pinging me, @afercia!

My question is whether this setting should be applied to every element in the block.

As far as I can tell, the "Contain text cursor inside block" setting in the Preferences panel is managed as keepCaretInsideBlock. This setting was added in #23546 and is respected on vertical and horizontal keyboard events.

Vertical movement:

isVertical &&
isVerticalEdge( target, isReverse ) &&
// When Alt is pressed, only intercept if the caret is also at
// the horizontal edge.
( altKey ? isHorizontalEdge( target, isReverseDir ) : true ) &&
! keepCaretInsideBlock

Horizontal movement:

isHorizontal &&
defaultView.getSelection().isCollapsed &&
isHorizontalEdge( target, isReverseDir ) &&
! keepCaretInsideBlock

Before these, however, there is a process that determines whether or not native behavior is preferred for a particular input element. Only the direction of the key and the element and its type are taken into account in this process, not the "Contain text cursor inside block" setting.

So I believe that a change like the one made in #43667 would affect the entire writing-flow regardless of that setting. From #23546, it appears that there is no specific discussion about whether or not to honor this setting in a particular input element.

@afercia
Copy link
Contributor Author

afercia commented Jul 21, 2023

@t-hamano thanks for your feedback.

My point is that native keyboard interaction with inputs should work as expected regardless of the "Contain text cursor inside block" setting. That applies to any of the expected interaction. Just let the browser do its job. For example:

  • Selecting all or part of the text within an input field via keyboard.
  • Moving within an input field with Left RIght arrow keys.
  • Home and End keys should move at the beginning and end of the text within the input.
  • Up and Down arrow keys should work as expected where necessary e.g. inputs of type number.
  • Etc. etc.

Since the list of expected behaviors is pretty long and I'm not even sure we would ever be able to list all the cases, I think the only best option is to just always preserve the native behavior.

Honestly, WritingFlow has always caused problems for keyboard interaction since when it was introduced in the editor. As an accessibility specialist, I'd love to see WritingFlow entirely removed as I think it does. more harms than goods. I know I may be biased but that's my honest professional opinion.

The "Contain text cursor inside block" setting was introduced as an attempt to remediate the problems introduced by WritingFlow. I don't think breaking native behaviors and then attempting to remediate is the best option for any web project. To me, WritingFlow is just a buggy attempt to make the editor behave like a Microsoft Word document, which in my opinion is a less than ideal design choice in the first place. Add to that WritingFlow is hard to maintain, hardly testable, and it will always cause problems even if we spend a considerable amount of time to try to address all the edge cases.

@spencerfinnell
Copy link

Thank you for investigating this @afercia and @t-hamano

Like @afercia, I believe all native elements should remain unaffected. I discovered this issue while developing a custom block UI that has a use for a native <input type="number" /> field.

@t-hamano
Copy link
Contributor

Okay, it certainly makes sense to respect the behavior of native elements. I will try to find time to work on this issue.

For now I have created code to test as many input elements as I can think of. If you run the following code in the browser console, you will be able to test the block temporarily.

Details
const el = wp.element.createElement;
const RichText = wp.blockEditor.RichText;
const useBlockProps = wp.blockEditor.useBlockProps;

wp.blocks.registerBlockType( 'test/native-input', {
	title: 'Native Input Test',
	edit: function ( props ) {
		return el(
			'div',
			useBlockProps( {
				style: {
					border: "3px #ccc solid",
					padding: "30px",
					display: "flex",
					gap: "10px",
					flexWrap: "wrap",
				}
			} ),
			el( 'input', { type: 'text', placeholder: 'text' } ),
			el( 'input', { type: 'tel', placeholder: 'tel' } ),
			el( 'input', { type: 'search', placeholder: 'search' } ),
			el( 'input', { type: 'email', placeholder: 'email' } ),
			el( 'input', { type: 'url', placeholder: 'url' } ),
			el( 'input', { type: 'number', placeholder: 'number' } ),
			el( 'input', { type: 'password', placeholder: 'password' } ),
			el( 'div', { contentEditable: true, style: { border: "1px #ccc solid", width: "100px" } } ),
			el( 'label', {}, el( 'input', { type: 'checkbox' } ), 'Checkbox' ),
			el( 'label', {}, el( 'input', { type: 'radio' } ), 'September' ),
			el( 'label', {}, el( 'input', { type: 'radio' } ), 'October' ),
			el( 'label', {}, el( 'input', { type: 'radio' } ), 'November' ),
			el( 'input', { type: 'color' } ),
			el( 'input', { type: 'date' } ),
			el( 'input', { type: 'datetime-local' } ),
			el( 'input', { type: 'month' } ),
			el( 'input', { type: 'week' } ),
			el( 'input', { type: 'time' } ),
			el( 'input', { type: 'file' } ),
			el( 'input', { type: 'reset' } ),
			el( 'input', { type: 'submit' } ),
			el( 'button', {}, 'Button' ),
			el( 'input', { type: 'button', value: 'Input type button' } ),
			el( 'textarea', { placeholder: 'textarea' } ),
		);
	},
} );

The new problem I noticed is that some elements lose focus when the Tab key is pressed. If you notice any new problems, please let me know.

67b9212d3b0672624426b38de9215bd7.mp4

@t-hamano t-hamano self-assigned this Jul 21, 2023
@ellatrix ellatrix linked a pull request Aug 18, 2023 that will close this issue
7 tasks
@ellatrix
Copy link
Member

This is something that #48256 should fix as well. I'll soon rewrite that PR.

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] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants