-
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
Distraction Free Mode: Don't show the metaboxes #48947
Conversation
! isDistractionFree && | ||
showBlockBreadcrumbs && | ||
! isMobileViewport && | ||
showBlockBreadcrumbs && | ||
isRichEditingEnabled && | ||
mode === 'visual' && ( |
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.
Nothing appears to have changed here, but the isDistractionFree
variable no longer contains the viewport width condition.
This fixes another problem. In trunk, there is a "gap" in the viewport width generated by this conditional statement, so the breadcrumb list is displayed when the viewport width is between 782px and 960px, even in distraction-free mode, as shown below.
87835d47b037b1cc67e9e3bfdcc5f478.mp4
e167fde
to
4ac5156
Compare
Size Change: -2 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
{ ( ! isDistractionFree || ! isLargeViewport ) && ( | ||
<EditorNotices /> | ||
) } |
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.
Does this fix another issue? Because it doesn't match isDistractionFreeMode && isLargeViewport
.
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.
If the original condition is rewritten as is, it will be ! ( isDistractionFree && isLargeViewport )
.
Isn't this the same as ( ! isDistractionFree || ! isLargeViewport )
? 🤔 Personally, I think this writing style is easier to read.
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.
I think we only want to hide the notices in "distraction-free" mode. We can remove the ! isLargeViewport
part.
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.
Your're right, I have confirmed that removing that section will produce the same expected results as before.
In addition, I have made the following changes in 77ab658 to address the following issues:
I noticed that the distraction-free mode could not be released unless the notice message bar was turned off, because it overlapped the header area. This may be a bug, but I think it should be addressed in a separate issue.
This should completely hide the notice message in distraction-free mode.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
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.
I would prefer if we address those issues separately. Let's just fix the metabox issue in this PR.
This reverts commit 77ab658.
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.
Thank you, @t-hamano! This works well in my tests.
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: e125710 |
Thank you @t-hamano 👍🏻 |
Fixes #48909
What?
This PR fixes a problem with the metabox panel when switching to distraction-free mode.
How?
In the Layout component of the post editor, the layout is controlled by the value of the
isDistractionFree
variable. However, this variable includes the viewport width as a condition, as shown below.This variable cannot be used because the metabox should be hidden in distraction-free mode, regardless of the width of the viewport. Also, it appears that the name of the variable does not exactly match what is intended by the name of the variable and the result of its value.
Therefore, I changed the name of the variable
isDistractionFreeMode
toisDistractionFree
, which purely determines the mode, and this variable now controls the display state of the metabox. At the same time, I adjusted the conditional statements in the areas affected by this change.Testing Instructions
In advance, run the following code in the browser console to display the notis messages.
Then, activate the Custom Fields panel from the Options menu.
Default Mode
Viewport width greater than 960px
Viewport width smaller than 960px
Viewport width smaller than 782px
Distraction Free Mode
I noticed that the distraction-free mode could not be released unless the notice message bar was turned off, because it overlapped the header area. This may be a bug, but I think it should be addressed in a separate issue.
Viewport width greater than 960px
Viewport width smaller than 960px
Viewport width smaller than 782px