-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add top toolbar to distraction free mode #56295
Conversation
Size Change: +116 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
This looks good—nice to see it's just a configuration check away. We should look into the editor setup flow soon. |
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.
Working well for me! I had distraction free on before loading this PR, so Idid have to toggle distraction free on/off to get the setting change to take place. I don't think this is worth addressing, but pointing it out just in case.
Looks like the tests only need updated.
@draganescu one issue is that if I toggle distraction free to test; don't like it; untoggle—I may think my toolbar is broken because I also need to toggle top toolbar off. |
@mtias ideally it should be in the state it was before distraction free was toggled on, but we don't store that anywhere. I wonder if the UX of testing DFM does not imply user understands how that options menu works? In case it implies this then the top toolbar is visibly toggled ON, just like DFM was before diciding they don't like it. It may be a very very niche issue? |
Following the merge of #55787 we're now able to render the top toolbar in the header of the editor UI when distraction free mode is active:
top-toolbar-distraction-free.mp4
I opted in this PR to always enable top toolbar when switching to distraction free mode. There is no reason to turn it off as the whole header auto hides.