-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Settings: Show message when Visual or Code editor are disabled #52737
Conversation
packages/edit-post/src/components/header/mode-switcher/index.js
Outdated
Show resolved
Hide resolved
Size Change: +408 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 6e2744c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5693420048
|
The notice helps, but my first thought is that is seems unnecessarily disruptive to force the user out of the editor to update this setting. My second thought is that the option feels a bit heavy-handed. Is there really a need for a personal preference that disables the visual editor? A different way to frame it could be to rename the option: "Make code editing the default writing mode". Then the option to toggle the visual editor could remain in the editor options. What do you think? |
@jameskoster Personally, I think it's a UX rabbit hole to try to address these issues, particularly because only a few users will ever see this state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not show a notice in the menu. Maybe instead of not showing the whole group the ModeSwitcher
could instead show a disabled menu option for Visual editor and instruct in the menu item choice info that to enable it the user should enable visual editing in their profile.
This seems unnecessarily long-winded. If there's intent to edit visually then ideally the user shouldn't have to jump through hoops to do so. They should be able to toggle right there. This is why I suggest updating the option to be about making the code editor the default. It's a simple label change that wont affect the experience for anyone who has toggled it on. But it also means the "Visual editor" option can appear in the editor tools, instead of being entirely disabled. Another benefit to this approach is that it 'fixes' a number of issues with the experience currently. For example with visual editing disabled:
|
@jameskoster Oh, I think I see what you're saying. It would be great to have a visual depiction of this, though. Would you be able to provide a few mockups? Also, should the user be able to 'Disable the visual editor when writing' again? Lastly, because "Disable the code editor" doesn't have any user-facing options (it can only be done with a filter), can you provide a mockup for that scenario?
@draganescu Any sense of how many things might break if we flip the |
@jameskoster Hm, I think changing the behavior of the 'Disable the visual editor when writing' setting is probably outside the scope of this PR... Can you provide design direction on simply showing a message instead? |
@jameskoster I imagine the change is way beyond a lanel given it's meant to disable a feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A design comment: I find the lack of vertical alignment caused by the left border and spacing of the notice particularly eye-disturbing.
I also agree that it's not semantic to directly render a notice inside a menu group.
I wonder if we can just render a MenuItem
with a disabled
prop and add some additional styling to it that makes it clearer that it's disabled, and/or any other additional style you deem necessary to make it more notice-like?
Yep. For clarity, I implemented a |
As discussed notices should be avoided in menus. We added one to the ellipsis menu in the block toolbar recently which had to be reverted after accessibility feedback. We should also avoid any custom styling for one-off use cases like this. To keep the PR as small as possible, I'd lean towards including the "Visual editor" menu item in a disabled state, potentially with a hint about how to re-enable the option: But it's not ideal:
This will likely be considered out of scope, but the only other option I can think of would be to include an "Enable visual editor" button which, when clicked would switch editing modes and update the "Disable the visual editor when writing" preference simultaneously. This would produce the most seamless UX for re-enabling visual editing. |
Thanks @jameskoster !
What do you think about italicizing the text to indicate disabled state? Is there some other visual approach we could use? |
The styles for the disabled state are baked into the |
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
@tyxla Could you apply the remaining type changes and changelog entry? Something is fubar in my local environment, and I'm not sure how long it will take to sort out. |
Happy to do it, but won't get to it before next week. |
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
@tyxla I managed to sort through my local environment issues and have applied your requested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me 👍 🚀 Nice work!
I'm formally not approving since there were other folks who requested changes, and I prefer to let them check it before approving.
@jameskoster @draganescu Want to give it another look? |
The changes around the visual editor option are good ✅ I'm not convinced the "Code editor is disabled for this site." help text is very helpful. It doesn't provide any more information beyond what is conveyed already be the disabled state. |
@jameskoster Is there some alternative you'd suggest? |
@jameskoster Cool, removed with 8214f40
This is covered in the "Why" of the PR description: When we remove an entire UI element, it can be confusing as to why it's missing. It's better to have some affordance that points them to the right direction. |
The issue I have is that merely disabling the menu item doesn't really "point people in the right direction". But I suppose it can trigger further investigation. Nonetheless it's not a detail we need to get hung up on in this PR. I think it's in a decent spot now! Thank you for entertaining my feedback :) |
Thanks @jameskoster !
Yep, this was my thought: give them enough of a breadcrumb to start taking action if they feel they need to. Looks like this PR is blocked by @draganescu's review now.. |
Asked by @danielbachhuber as seems that @draganescu is AFK and cannot re-review.
Fixes #18351
What?
Shows a 'Visual editor is disabled.' message when the Visual or Code editor are disabled:
Visual editor disabled
Code editor disabled
Both editors available
Why?
While doing support last week, I came across a user who was confused as to why they couldn't get back to the Visual editor. As it turns out, they had checked the 'Disable the visual editor when writing' setting in their profile.
When we remove an entire UI element, it can be confusing as to why it's missing. It's better to have some affordance that points them to the right direction.
Additionally, allowing the user to disable/enable the visual editor from this menu opens a UX can of worms. I decided to keep things simple by only adding a message.
How?
For this particular condition, returns a message from
<ModeSwitcher>
instead of null.Testing Instructions
Disabling the Visual editor
/wp-admin/profile.php
.Disabling the Code editor
Introduced in #14932 but no user-facing option.