Add default restoration of UI when exiting distraction free mode #58455
+56
−41
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
…restore toolbar to non fixed
|
Size Change: -119 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
draganescu
added
[Type] Bug
draganescu
requested review from
Mamaduka,
getdave,
richtabor and
MaggieCabrera
Mamaduka
requested changes
Jan 30, 2024
|
Here's what I'm getting in the post and site editors: dfm-posteditor.mp4 |
|
There definitely is something wrong with my computer :D Edit: nope, the problem is in front of the computer. I was testing using cmd shit \ kbd shortcut, and I think that still has the calls to close the panels. I'll try to keep the code change and lean exclusively on the actions instead of duplicating all the proceduress all over the place. |
…ers that also toggle, fixedbug with undo link in snackbar
|
@Mamaduka @richtabor this is ready for a new spin:
|
Mamaduka
reviewed
Feb 1, 2024
There was a problem hiding this comment.
Thanks for the update, @draganescu!
I think we're almost there; the missing part is the general sidebar — the sidebar doesn't re-open after exiting the DFM.
The sidebar state is a user preference that persists across browsers.
| @@ -59,6 +45,7 @@ function WritingMenu() { | |||
| <PreferenceToggleMenuItem | |||
| scope="core" | |||
| name="distractionFree" | |||
| handleToggling={ false } | |||
There was a problem hiding this comment.
Nit: introducing a new public prop/API for a single use case feels odd. Should we mark it as unstable or experimental?
There was a problem hiding this comment.
Maybe we should yes, but I think the ability to handle the toggling in the onToggle is not that farfetched.
There was a problem hiding this comment.
Maybe default toggle shouldn't run when onToggle callback is defined - onToggle ? onToggle() : toggle()
cc @talldan
There was a problem hiding this comment.
I don't think this is right, as the main purpose of this component is a thin wrapper around MenuItem that toggles the preference in the preference store, and this code circumvents the one thing it does.
I think an alternative would be to implement it using a MenuItem. There's only a spoken message that has to be output, and the distraction free menu item provides its own custom messages anyway, so that logic isn't complicated.
Seems like it'll end up being a very similar amount of code just using a MenuItem.
|
@Mamaduka sidebars didn't open before. Should we introduce this with this PR? Also which one, the block inspector or the list view? |
Mamaduka
approved these changes
Feb 1, 2024
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core SVNIf you're a Core Committer, use this list when committing to GitHub Merge commitsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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?
Closes #56565
Why?
Because block breadcrumbs are a distraction and aso it is a bit disorientating to have the top toolbar remain fixed if you had it floating before entering distraction free mode.
How?
Testing Instructions
Testing Instructions for Keyboard
N/A
Screenshots or screencast
dfm-breadcrumb-fix.mp4