WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.45k stars 4.17k forks source link

[Tracking] Command Palette: Turn off DFM mode when a command not suitable for DFM mode is executed #53993

Closed t-hamano closed 1 year ago

t-hamano commented 1 year ago

What problem does this address?

The problem was discovered from this comment

Many commands do not take into account whether or not you are currently in DFM mode. As a result, unintended commands may be executed in DFM mode, causing layout corruption, etc.

What is your proposed solution?

As far as I could find out, at least the following commands should not be displayed in DFM mode.

Update: It seems better to leave these commands in place and remove DFM mode when they are executed.

Post Editor

Site Editor

richtabor commented 1 year ago

Are you thinking of a way to omit from DFM (maybe an allowed list?), or adding a check in each command? The latter would be tough to mitigate as we continue adding commands.

t-hamano commented 1 year ago

Are you thinking of a way to omit from DFM (maybe an allowed list?), or adding a check in each command? The latter would be tough to mitigate as we continue adding commands.

Not only in DFM mode, I think various conditions must be checked to see if the command is displayed. Therefore, I think we have to take the latter approach. The following recently submitted PRs are examples.

ntsekouras commented 1 year ago

As far as I could find out, at least the following commands should not be displayed in DFM mode.

Should we hide these commands in DFM mode or just disable the mode? 🤔 Any thoughts @draganescu ?

draganescu commented 1 year ago

So far we littered the codebase with code that disables DFM when the UI is no longer conforming to DFM mode.

As an UX I think so far it's the right path. As an implementation it's hard to discern if at the root is better (less flexible but prevents these UI bugs once and for all) or at the consumer (it's a few more lines to always toggle DFM off).

I think as far as UX goes we should leave the commands there and just turn off DFM for everything that causes UI bugs mostly because we want to improve DFM to a level where it won't have to be turned off anymore.

t-hamano commented 1 year ago

Thanks for the advice, @draganescu ! I have changed the title of the issue.

BTW, #52522 has also been reported that the Settings/Styles panel is unintentionally displayed in DFM mode.

ntsekouras commented 1 year ago

I'll have a PR soon that handles most of the cases.

ntsekouras commented 1 year ago

Reset styles to defaults doesn't open a sidebar but the changes aren't saved and this might not be obvious.

Hide block breadcrumbs / Show block breadcrumbs: The breadcrumb is not always displayed in DFM mode. Is this command necessary?

I think breadcrumbs doesn't affect anything with DFM..

t-hamano commented 1 year ago

Thank you for addressing this issue, @ntsekouras!

Does this mean that this issue can be closed since DFM does not need to be considered for the following commands?

ntsekouras commented 1 year ago

I think we can close this now, yes.