facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
17.5k stars 1.45k forks source link

Feature: allow rich-text editor Tab-to-Indent to be disabled #2854

Closed iansjk closed 1 year ago

iansjk commented 1 year ago

Hi there!

We're looking into lexical as part of a feature allowing students to compose rich text responses to an online textbook. However, we're also trying to pick a rich-text editor that will be keyboard accessible.

The problem we're having is that because Tab and Shift-Tab are captured in the editor (to provide indent/unindent support), once the RTE has focus it's not possible to escape it with Tab/Shift-Tab, making other elements on the page inaccessible by keyboard or switch devices.

Could we get an option to disable this indent/unindent behavior for Lexical rich-text editor instances? The expectation is that pressing Tab while inside of the RTE will go to the next focusable item, and pressing Shift-Tab will go to the previous focusable item.

iansjk commented 1 year ago

It looks like the relevant handler is here: https://github.com/facebook/lexical/blob/d67a9c36ba4b73625e47cb553650dac29e6941a7/packages/lexical-rich-text/src/index.ts#L808-L822

Which I believe is added by default when the RTE package is used?

I understand that it should be possible to bind a different KEY_TAB_COMMAND handler with a higher priority than this one, but that would also entail having to explicitly focus the next element in the tab sequence (or previous element in the tab sequence, if Shift-Tab is used).

acywatson commented 1 year ago

It looks like the relevant handler is here:

https://github.com/facebook/lexical/blob/d67a9c36ba4b73625e47cb553650dac29e6941a7/packages/lexical-rich-text/src/index.ts#L808-L822

Which I believe is added by default when the RTE package is used?

I understand that it should be possible to bind a different KEY_TAB_COMMAND handler with a higher priority than this one, but that would also entail having to explicitly focus the next element in the tab sequence (or previous element in the tab sequence, if Shift-Tab is used).

I might be mistaken, but I think if you get rid of that event.preventDefault, tab navigation should work normally.

editor.registerCommand<KeyboardEvent>( 
   KEY_TAB_COMMAND, 
   (event) => { 
     //noop
   }, 
   COMMAND_PRIORITY_HIGH, 
 ), 

Does that not work as expected for you?

thegreatercurve commented 1 year ago

@acywatson The event just then bubbles, and you indent the content once and then tab out of the editor.

My vote is to strip it out of the initial RTE set up altogether. We definitely don't want users having to proactively make their editor accessible by adding in a KEY_TAB_COMMAND command listener with a higher priority.

If they want the tabbing behaviour, and don't need to care about accessibility in terns of tabbing in and our of the root element (i.e. if the editor is the only thing on the page), they can add it themselves in a plugin.

Thanks for the report @iansjk! This is a really important accessibility issue. 🙏

thegreatercurve commented 1 year ago

PR which does the above: #2855

iansjk commented 1 year ago

Thanks for the response @thegreatercurve! There's other Lexical accessibility issues that our accessibility specialist identified–I'll be happy to report those as I collect more info about them.

thegreatercurve commented 1 year ago

@iansjk I would love that! This is something I care about a lot, so any issues you report, I'll aim to prioritise straight away.

We do our own internal accessibility audits, and we know we perform better than Draft, but there's always things we can be doing better. This is where these kinds of OSS contributions are invaluable.

Thanks again 🙏

acywatson commented 1 year ago

@acywatson The event just then bubbles, and you indent the content once and then tab out of the editor.

I think you misunderstood - you need to noop the whole listener, as in my example.

GermanJablo commented 1 year ago

My vote is to strip it out of the initial RTE set up altogether.

Mine is to leave it as a separate plugin or as a boolean property of the RTE plugin that can be activated at will by each user. Let each one decide please.

iansjk commented 1 year ago

If they want the tabbing behaviour, and don't need to care about accessibility in terns of tabbing in and our of the root element (i.e. if the editor is the only thing on the page)...

(Note that even in this circumstance, normally one should be able to tab into the browser chrome once the tabbable items in the webpage are exhausted.)

echarles commented 1 year ago

I think you misunderstood - you need to noop the whole listener, as in my example.

I have tested the snipper, and I need to return true to disable.

editor.registerCommand<KeyboardEvent>( 
   KEY_TAB_COMMAND, 
   (event) => true, 
   COMMAND_PRIORITY_HIGH, 
 ), 

Mine is to leave it as a separate plugin or as a boolean property of the RTE plugin that can be activated at will by each user. Let each one decide please.

This is also the first though I had.

Awjin commented 1 year ago

I think it's worth mentioning that the W3C guidance on tab capturing in rich text editors is not well-specified: https://github.com/w3c/aria-practices/issues/88.

Current practices that seem common are ctrl-m or esc to toggle tab capturing.

acywatson commented 1 year ago

Closing this, since it's totally possible to disable this functionality now. We'll continue discussing the what the default behavior should be and the path for on the PR.

Clobbe commented 1 year ago

@acywatson - I've read this thread and am facing this issue in Lexical for React. Will this be pushed to React-version of Lexical as well?

guicoelho commented 1 year ago

Been wondering for hours why tab indentation stopped working on my editor. I agree with the fix, just wondering if there's a better way to keep up with these changes to make sure my implementation doesn't break. Suggestions welcome.

If you want to use TAB to indent again, you need to use the plugin that's also part of #2855. In react that would be:

    import { TabIndentationPlugin } from '@lexical/react/LexicalTabIndentationPlugin';
Awjin commented 1 year ago

That's the nature of using pre-v1.0 software. I'd recommend subscribing to this repo for release notifications.