-
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
Disable distraction free prefference effects on small viewports #45591
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
Size Change: +8 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
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 agree with the direction here.
Whilst as you mention it's not guaranteed that smaller viewports don't use a keyboard or mouse, it's pretty likely that is the case.
Therefore this seems like a good interim fix for the bug whereby you enabled distraction free mode on your larger viewport and then got into an impossible editing situation on your small screen device (due to synced editor preferences).
I haven't yet been able to manually test this works.
@@ -40,13 +41,12 @@ function Header( { setEntitiesSavedStatesCallback } ) { | |||
showIconLabels: | |||
select( editPostStore ).isFeatureActive( 'showIconLabels' ), | |||
isDistractionFree: | |||
select( editPostStore ).isFeatureActive( 'distractionFree' ), | |||
select( editPostStore ).isFeatureActive( 'distractionFree' ) && | |||
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.
[] | |
[ isLargeViewport ] |
You'll want to re-run the selector if the viewport changes right?
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 seems still unfixed? Can we commit that?
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 raised #47611
@@ -116,7 +117,8 @@ function Layout( { styles } ) { | |||
showIconLabels: | |||
select( editPostStore ).isFeatureActive( 'showIconLabels' ), | |||
isDistractionFree: | |||
select( editPostStore ).isFeatureActive( 'distractionFree' ), | |||
select( editPostStore ).isFeatureActive( 'distractionFree' ) && | |||
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.
Should isLargeViewport
be a dependency of the useSelect
in order that it will re-run when the viewport 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.
same here
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 raised #47611
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.
What?
This tiny PR disables the effects of distraction free mode when using a small viewport such as a mobile phone.
Why?
In distraction free mode the top toolbar is invisible and one cannot save or publish. The way to get it back is via keyboard and mouse and small viewports usually have none.
How?
Ignore the prefference if the viewport is not at least "large".
Testing Instructions
Screenshots or screencast
Todo: