Introduce distraction free mode #41740
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
draganescu
added
[Type] Enhancement
|
Size Change: +2.22 kB (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
|
This looks like a good start to me. Since it's one part of some other "interface personalisation" ideas, maybe we should open a tracking issue that encapsulates everything? In terms of this PR specifically, I don't think that we necessarily need to disable other view options when the interface is hidden. It doesn't seem harmful to allow folks to toggle the top toolbar option for example. It would also be good if the top toolbar remained visible when the ellipsis menu is open. Edit: It would be good to restore the animations from #38928 (comment) as well, and refine. The notion of elements moving in and out of the viewport (rather than simply fading off the canvas) will feel more cohesive when we consider other views like the zoomed out one, and browse mode. |
|
I think it would be good to land this as an infrastructure base to build on. In the exploration PR there was a lot of back and forth to this point and even now the size of the change is considerable. So I wonder what is the minimum lacking here to land this and then start refining? |
|
I'd like to check this out for at least basic A11Y. I'll be around shortly. |
draganescu
force-pushed
the
add/distraction-free-mode
branch
from
3ead5c4
to
548cda1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
draganescu
force-pushed
the
add/distraction-free-mode
branch
from
f847053
to
8a09c31
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
It looks like a very good start! One thing. What about adding an X in the top left corner to signal an escape out of this mode? Mockup
|
draganescu
force-pushed
the
add/distraction-free-mode
branch
from
af1f531
to
353c839
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
talldan
reviewed
Oct 10, 2022
talldan
reviewed
Oct 10, 2022
Use the store directly instead. Co-authored-by: Daniel Richards <677833+talldan@users.noreply.github.com>
Co-authored-by: Ben Dwyer <275961+scruffian@users.noreply.github.com>
|
I have merged this but it is not prime time ready yet. The PR:
It still requires a few updates to be at a level on par with other Gutenberg UX:
Worst case scenario this could be reverted to an experiment. |
|
Also, since the menu option is a checkbox, the menu should not close after enabling from the Options menu. It is considered bad UX to have one control in a menu act differently than all the others. I'm not really interested as to why this couldn't get done as long as it gets done in the future. Thanks. |
|
It's a very nice feature, well done on getting it to this stage. Another thing around the options menu (and also other dropdowns for consistency), It might be worth looking at keeping the top bar visible when focus is in the options menu dropdown, as the way it animates can lead to users clicking on the wrong thing: Kapture.2022-10-11.at.11.14.49.mp4 |
5 tasks
youknowriad
reviewed
Oct 17, 2022
packages/block-editor/src/components/block-list/use-in-between-inserter.js
Show resolved
Hide resolved
| @@ -23,13 +24,12 @@ import MainDashboardButton from './main-dashboard-button'; | |||
| import { store as editPostStore } from '../../store'; | |||
| import TemplateTitle from './template-title'; | |||
|
|
|||
| function Header( { setEntitiesSavedStatesCallback } ) { | |||
| function Header( { setEntitiesSavedStatesCallback, isDistractionFree } ) { | |||
There was a problem hiding this comment.
Why is this a new prop instead of just using useSelect to retrieve it?
| @@ -195,10 +201,14 @@ function wrapperSelector( select ) { | |||
| ?.__experimentalCaptureToolbars | |||
| ); | |||
|
|
|||
| const settings = getSettings(); | |||
There was a problem hiding this comment.
getSettings called here doesn't cause a re-render if the settings change, it should probably be called inside useSelect instead to avoid bugs.
| @@ -36,7 +36,7 @@ const MoreMenu = ( { showIconLabels } ) => { | |||
| scope="core/edit-post" | |||
| /> | |||
| ) } | |||
| <WritingMenu /> | |||
| <WritingMenu onClose={ onClose } /> | |||
There was a problem hiding this comment.
How is this change related to this PR?
There was a problem hiding this comment.
Thanks for the catch up review! 🙏🏻 This change - which I'll probably undo, idk - is to close the writing menu when entering this mode, because the top toolbar hides and this menu remains orphan in the UI.
| @@ -20,6 +20,8 @@ export default function PreferenceToggleMenuItem( { | |||
| messageActivated, | |||
| messageDeactivated, | |||
| shortcut, | |||
| toggleHandler = () => null, | |||
There was a problem hiding this comment.
maybe onToggle is a better name here?
| <PreferenceToggleMenuItem | ||
| scope="core/edit-post" | ||
| name="distractionFree" | ||
| toggleHandler={ toggleDistractionFree } |
There was a problem hiding this comment.
Why do we need this handler at all? I thought the PreferenceToggleMenuItem component already toggled the preference automatically?
There was a problem hiding this comment.
Because we need other stuff to happen (e.g. close sidebars) not only to toggle.
There was a problem hiding this comment.
But why is this different than the other preference toggles above? the sidebars should close when the preference is set anyway.
There was a problem hiding this comment.
Yes. I did not alter the rendering of everything distraction free should close so that they render conditionally on the preference. Instead I created this cleanup handler. To have:
- the top toolbar disappear
- the inserter close
- the list view close
- the general sidebar close
- the 1st block focused
... when this preference is toggled on I'd have to pepper with checks for the isDistractionFree in all these places (the last one would probably still need a special handler).
I think the answer to:
why is this different than the other preference toggles above?
is that distraction free has effects across the UI along the lines of availability not only behavior.
mcsf
reviewed
Oct 24, 2022
This was referenced Mar 1, 2023
This was referenced Mar 30, 2023
|
Hi @talldan and @alexstine I'll be looking into solving the problem described by Alex via the idea from Dan, to keep the header toolbar "visible" when the focus is in the writing menu. If I can do that, then I don't need to close the writing menu, hence solve Alex's problem too. |
|
The gist is this:
I have tried to solve this but have not had any success so far with convincing motion to kick in properly without rerendering the whole header component. I'll keep trying. PS I know @alexstine said " I'm not really interested as to why this couldn't get done as long as it gets done in the future" - but this is the future and I can't do it :))) there has to be a way, so far I am thinking of just trying with CSS again but framer motion does the easing so much better. |
|
@draganescu As a hot fix, can you simply add some type of |
How come, in the code we manually focus the 1st block in the editor specifically to not lose focus? Opening the more menu dropdown and focus one specific menu item after the header is remounted seems quite convoluted. There has to be a way to avoid the need to re-mounting. |
|
@draganescu To replicate:
It seems this only happens when distraction free is disabled so my mistake if this wasn't clear. Thanks. |
|
@draganescu @alexstine I've put a fix together for the focus loss issue in this PR - #51627. It seems to resolve the issue in the majority of cases, though I am still seeing a focus loss on the rare occasion. |
|
I've updated and approved that PR 👏🏻 |
|
Awesome, having a look now. 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR is the result of the exploration in #38928 to introduce a
distraction free mode. This mode build on top of the reduce interface
preference, amplifying its effect.
Why?
To enhance content production in WordPress. Text based content is one of
the most important forms of content. While the site and page building
enhancements make multi media content production much easier, through the
visual tools offered by the block editor, text content production can
sometimes feel impaired.
A distraction free mode (the term became somewhat of an industry standard)
aims to remove visual clutter when the focus is on text based content
production.
How?
The PR builds on the reduce interface prefference. Until now the
preference, when toggled on, would only hide some of the toolbar icons and
a few other non-essential UI items. With this PR the prefference will have
a more drastic effect, turning the UI into a blank page:
area
Known issues
distraction free mode is on and the
ESCkey is pressedbreak some things
hasReducedUiprops need to be refactored intoisDistractionFreeTesting Instructions
Screenshots or screencast
distraction-free-motion.mp4