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

Edit mode: arrow up/down should not move to other blocks #22204

Closed
wants to merge 8 commits into from
Closed

Edit mode: arrow up/down should not move to other blocks #22204

wants to merge 8 commits into from

Conversation

alexstine
Copy link
Contributor

@alexstine alexstine commented May 8, 2020

Description

All details in #22190.

How has this been tested?

I tested in WordPress version 5.4.1 with a built Gutenberg test plugin. I used NV Access on Windows 7 for the accessibility testing.

Types of changes

Breaking change that will only make it possible to switch blocks in navigation mode instead of edit mode.

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.

My first PR, hoping I covered all bases on my proposal for this change.

@gziolo gziolo requested review from enriquesanchez and afercia May 8, 2020 05:54
@gziolo gziolo requested review from MarcoZehe and removed request for youknowriad, afercia and enriquesanchez May 8, 2020 05:55
Copy link
Contributor

@MarcoZehe MarcoZehe left a comment

Choose a reason for hiding this comment

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

There are several failing end-to-end tests with this PR. Please look at these and deal with the fall-out. The failing tests may uncover unforeseen side effects with this change.

@ellatrix
Copy link
Member

ellatrix commented May 8, 2020

Right, we cannot just remove this feature now. Many people are expecting vertical arrow keys to navigate between blocks. Is there a way we can announce the switch to another block? Or any other was to make it clear that arrow up/down will leave the current block?

@MarcoZehe
Copy link
Contributor

Again, the combinations I tested all announced the shift to a different block as a normal focus change. So the fact that the block was moved out of was clearly communicated. Granted, it may be unexpected if you cannot see that you're on the last line. However, I don't see how we can add a warning message for screen readers without interrupting the line reading when moving to the first or last line in a block.

I am really not happy with this situation, either, but agree that breaking vertical navigation in edit mode between blocks is going to break a lot of keyboard users.

@ellatrix
Copy link
Member

ellatrix commented May 8, 2020

I'm not sure I fully understand the problem. You can undo the navigation by pressing the opposite button?

@alexstine
Copy link
Contributor Author

I'm going to try to figure out how to fix the PR.

I think it is important to do this because of the unexpected block changes. It's simply bad UX for somebody with a screen reader. Anyone who reads a text field with a screen reader uses the arrows up/down keys. If a user thinks they have more text and presses down, they could be thrown in to another block. Yes it is announced, but this is not how text fields are supposed to behave.

Can I venture another question? If blocks can be transitioned currently with the arrows, why even have a navigation mode to maintain?

This is basically the same reason NV Access has browse/edit mode. You cannot do the same things in both modes, following distinct navigation and edit mode is the way to go.

@ellatrix
Copy link
Member

ellatrix commented May 9, 2020

Navigation between these modes is very different. In navigation mode you go through a block with each press, in edit mode you go through every editable line, where the Carey is additionally place at the same horizontal position if possible. Arrow navigation in edit mode should be seen as navigating a document where the block boundaries are blurred.

@alexstine
Copy link
Contributor Author

@ellatrix Yes, this is kind of the point I was after.

"Arrow navigation in edit mode should be seen as navigating a document where the block boundaries are blurred."

I disagree. I think there needs to be a clear distinction between edit/navigation mode. Take my point about how NV Access screen reader works. When you are in browse mode, you cannot type in to an edit field and when you are in edit mode, you cannot browse the web page like you would in browse mode. That's the way this feature should be implemented, why, because it's much better UX this way for people that use screen readers.

I understand the user will hear a clear message when switching to another block, but it would frustrate me if I'm typing a paragraph and hit the down arrow too many times only to find myself 1-2 blocks away from where I wanted to edit in the first place.

It would make the block switching take a bit longer, this I understand. However, it would provide a much needed separation between the 2 modes which would drastically improve my ability to use the editor.

I'm looking at this from a blind developer/user perspective. I've also used the computer both ways with a mouse and without when I had vision a few years back. I'm not so worried about keyboard navigation for people with vision or even low vision, all it takes is a simple mouse click to change to another block. I am also fairly certain that when you enter navigation mode, there are visual queues to show which block is selected and this selection moves up and down based on Tab or Shift+Tab. Please correct me on that point if I'm wrong.

At the end of the day I understand my way will make it longer to switch blocks, but it will be a much more accessible and reliable way to manage this compared to switching when the user may not want to in edit mode.

Thoughts? Worth keeping this open?

PS: still trying to figure out how to fix the PR, not really understanding why it's getting the CI errors.

Thanks, Alex

@MarcoZehe
Copy link
Contributor

After sleeping on it for a night, I am warming up to the idea. What Gutenberg is right now is a mixed bag between a conventional editor and a design tool that is essentially focused on a block at a time. Simply being able to navigate between blocks with the arrow keys when in edit mode is inconsequential. This is the „trying to be a normal editor“ part, which really no longer works with Gutenberg the more complex it and its blocks get. Granted, Enter from one block should still start a new one as before, but containing vertical navigation in edit mode to a single block does make sense if going all-in on „this is focusing on a single block“. Even visually, this happens. So it makes sense to do it from a keyboard UX perspective, too.

And as I said in the related issue: Gutenberg for mobile already does that. You cannot transfer keyboard input focus from one block to another there, either. There, the „focusing on a single block“ paradigm is consequentially enforced.

@afercia
Copy link
Contributor

afercia commented May 9, 2020

containing vertical navigation in edit mode to a single block does make sense

I'd totally second that. I'd also like to quote what I said in the related issue:

in the initial accessibility recommendations posted on March 2017, it was recommended to treat each block as a sort of "modal", at least in terms of interaction. See #297
The block concept has evolved since then but it still makes a lot of sense to provide a standard arrows key behavior while editing a block. That is: at the very least, disable "Writing Flow" while editing a block.

Maybe after three years, and after the switch between Navigation and Edit mode has been implemented, it's finally time to come to some consensus on further differentiating the navigation experience from the editing one.

By "modal behavior" we certainly didn't mean a modal dialog. Instead, it's about making the experience of editing a block something that focuses on that specific block, honoring the expected, native, keyboard interaction that all input fields have. That means all the keys combinations that work in native input fields to move through the input content, select parts of the content, etc., should work in the Gutenberg editable fields as well.

@ellatrix I think in the past we already excluded left and right arrows from triggering the WritingFlow behavior. We should now just completely disable WritingFlow when a block is in Edit mode.

Not to mention there are already several places in the codebase where WritingFlow is prevented from kicking-in because it would otherwise break interaction with other components. I would argue it is WritingFlow that introduces a non-standard behavior that is proving more and more over time to be difficult to maintain and prone to break other expected behaviors in the user interface.

@ellatrix
Copy link
Member

ellatrix commented May 9, 2020

We don’t exclude left and right. This should work the same way.

@afercia
Copy link
Contributor

afercia commented May 12, 2020

We don’t exclude left and right. This should work the same way.

We do. The following code is what I meant:

// Native inputs should not navigate horizontally.
const { tagName } = element;
return tagName !== 'INPUT' && tagName !== 'TEXTAREA';

If I remember correctly, this prevents WritingFlow from kicking in when a native input is focused and a modifier key is used. Previously, key combinations like Cmd or Ctrl + Left Arrow were broken within input fields.

We should do the same for any UI that is communicated and works as an input field / textarea, to preserve native keyboard shortcuts behavior and expected interaction.

@ellatrix
Copy link
Member

What I meant is that we did not disable it horizontally. If you press arrow right at the end of a paragraph, focus moves to the next paragraph.

@alexstine
Copy link
Contributor Author

I should have also disabled this horizontally. I didn't realize it switched blocks by horizontal navigation. The problem with horizontal navigation is if a user is reading character by character. User finds the end of a sentence but can see visually that they are at the end, thrown in to another block. Equally bad UX, different situation.

Writing flow is great if you can see where the cursor is, but there's too much movement that could happen unexpectedly using a screen reader.

Will push an update later today.

Thanks.

@alexstine
Copy link
Contributor Author

Updated the PR. Horizontal navigation is now disabled. Now edit mode remains a restrictive feature, navigation mode needs to be used to switch blocks.

Shift+Tab to go to block toolbar and Tab to go to settings is still intact and working well.

Thanks.

@afercia
Copy link
Contributor

afercia commented May 17, 2020

I realize the discussion on this PR is mixing up technical details with arguments on the opportunity to make such a change. It's something I contributed to myself and I'm sorry for that. Please let's try to move general discussion to the related issue #22190.

I have to say that what I'd expect on any PR is the team to be welcoming towards new contributors and guide them to the best technical implementation possible. Once finalized, the PR can be discussed in a more appropriate place with a broader audience, as in WordPress no single developer is entitled to make decisions.

Instead, I'm sorry to say that what I'm seeing here on this PR is mostly a dismissive reaction which is not welcoming at all.

@mtias
Copy link
Member

mtias commented Jun 4, 2020

Let's aim to wrap this functionality in a setting so that we can merge it and continue to explore surfacing these configurations in the "welcome flow".

@enriquesanchez
Copy link
Contributor

What would be a good way to call this setting? I'm assuming 'Writing flow' is not intuitive or descriptive enough for most users to understand what's it about.

Related, the Accessibility team has been discussing ways to document and communicate accessibility-related features and settings. When this becomes a setting, this would be a good one to add to that list.

I also think the welcome flow would be a good time to show these.

@mtias
Copy link
Member

mtias commented Jun 5, 2020

Not sure about the name. There's another in progress features (the one that toggles icon labels across the header UI) that would also be good to group.

@afercia
Copy link
Contributor

afercia commented Jun 6, 2020

Re: the name: Firefox has a similar native fature called "Caret browsing", not that intuitive or appropriate for the block editor though. Press F7 on Firefox to see it in action.

@draganescu draganescu added the Needs Copy Review Needs review of user-facing copy (language, phrasing) label Jun 23, 2020
@draganescu
Copy link
Contributor

Howdy! It appears we're stuck on name here and also the place where this should be located as a setting. Let's therefore proceed with a simple and descriptive thing, say "Keep caret inside blocks" which can then be improved to sound less "tech-y".

The location of the setting should be the options panel that is accessible via the ellipsis menu on the top right corner of the editor:

Screenshot 2020-06-23 at 11 25 44

@alexstine do you think you can continue this PR in this fashion:

  • restore the deleted core
  • implement the option (default false) as an item of the options panel
  • check for that option and if it is ON (true) disable the Writing Flow functionality by conditionally not executing the code you removed in this PR
  • write one test for when this setting is ON (all the existing tests should work with the setting OFF)

It would be nice if this can exist before the 1st beta of WP 5.5 as there still is potential to land it in the next WP release. If you could use some help we could try a fork of this PR and carry it on over there.

@michelleweber
Copy link

Copy-wise, you can never really go wrong with simple and descriptive. Would "Keep caret inside active block" make it a smidge clearer?

@enriquesanchez
Copy link
Contributor

"Keep caret inside active block" sounds pretty clear to me, great suggestion @michelleweber!

Something about having this setting next to 'Pre-publish checks' feels odd to me. Considering that we're working on other related features...

There's another in progress features (the one that toggles icon labels across the header UI) that would also be good to group.

...would it maybe be a good idea to create a new section inside the Options menu? Maybe something like 'Personalization'?

@mtias
Copy link
Member

mtias commented Jun 24, 2020

"Personalization" seems extremely vague to me. I'd either call it "Writing" or "Accessibility" if we are going to group more features there (the show labels, etc).

@afercia
Copy link
Contributor

afercia commented Jun 24, 2020

I'd recommend to not use "Accessibility" as the Options new section name. This is a setting that can be useful to all users, depending on their needs or preferences.

@mtias
Copy link
Member

mtias commented Jun 24, 2020

Accessibility shouldn't imply that it's only for a subset of users, no? For example, macOS includes a lot of widely useful features in its accessibility panel which are about adapting the interface to your individual needs.

@enriquesanchez
Copy link
Contributor

"Personalization" seems extremely vague to me

That is fair and I agree. Naming things is hard, so I'll add more ideas to the pool with the hopes that it helps us make a connection and come up with something better:

How about 'Editor experience'?

@afercia
Copy link
Contributor

afercia commented Jun 25, 2020

Since it's a feature related to keyboard, I'd call the section "Keyboard options" or something along those lines. More settings related to keyboard interaction may come in the future.

Also, I'm not sure users are familiar with the term caret. Wouldn't cursor or text cursor be more understandable, though technically not 100% correct? https://en.wikipedia.org/wiki/Caret_navigation

@tellthemachines
Copy link
Contributor

@alexstine, and everyone following this discussion, I just created #23546 to add an option that disables caret browsing in edit mode. I tried to follow all the advice from this PR and from #22190. Feedback welcome!

@alexstine
Copy link
Contributor Author

@tellthemachines Thanks, left a comment on the other PR, closing this one out.

@alexstine alexstine closed this Jun 30, 2020
@alexstine alexstine deleted the fix/block-arrows branch June 30, 2020 22:31
@priethor priethor added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Copy Review Needs review of user-facing copy (language, phrasing)
Projects
None yet
Development

Successfully merging this pull request may close these issues.