-
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
Editor: Unify device preview styles #56904
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import { | |
useSettings, | ||
__experimentalRecursionProvider as RecursionProvider, | ||
privateApis as blockEditorPrivateApis, | ||
__experimentalUseResizeCanvas as useResizeCanvas, | ||
} from '@wordpress/block-editor'; | ||
import { useEffect, useRef, useMemo, forwardRef } from '@wordpress/element'; | ||
import { useSelect } from '@wordpress/data'; | ||
|
@@ -90,14 +91,18 @@ function EditorCanvas( | |
editedPostTemplate = {}, | ||
wrapperBlockName, | ||
wrapperUniqueId, | ||
deviceType, | ||
} = useSelect( ( select ) => { | ||
const { | ||
getCurrentPostId, | ||
getCurrentPostType, | ||
getCurrentTemplateId, | ||
getEditorSettings, | ||
getRenderingMode, | ||
getDeviceType, | ||
} = select( editorStore ); | ||
const { getPostType, canUser, getEditedEntityRecord } = | ||
select( coreStore ); | ||
const postTypeSlug = getCurrentPostType(); | ||
const _renderingMode = getRenderingMode(); | ||
let _wrapperBlockName; | ||
|
@@ -110,14 +115,11 @@ function EditorCanvas( | |
|
||
const editorSettings = getEditorSettings(); | ||
const supportsTemplateMode = editorSettings.supportsTemplateMode; | ||
const postType = select( coreStore ).getPostType( postTypeSlug ); | ||
const canEditTemplate = select( coreStore ).canUser( | ||
'create', | ||
'templates' | ||
); | ||
const postType = getPostType( postTypeSlug ); | ||
const canEditTemplate = canUser( 'create', 'templates' ); | ||
const currentTemplateId = getCurrentTemplateId(); | ||
const template = currentTemplateId | ||
? select( coreStore ).getEditedEntityRecord( | ||
? getEditedEntityRecord( | ||
'postType', | ||
'wp_template', | ||
currentTemplateId | ||
|
@@ -135,6 +137,7 @@ function EditorCanvas( | |
: undefined, | ||
wrapperBlockName: _wrapperBlockName, | ||
wrapperUniqueId: getCurrentPostId(), | ||
deviceType: getDeviceType(), | ||
}; | ||
}, [] ); | ||
const { isCleanNewPost } = useSelect( editorStore ); | ||
|
@@ -152,6 +155,7 @@ function EditorCanvas( | |
}; | ||
}, [] ); | ||
|
||
const deviceStyles = useResizeCanvas( deviceType ); | ||
const [ globalLayoutSettings ] = useSettings( 'layout' ); | ||
|
||
// fallbackLayout is used if there is no Post Content, | ||
|
@@ -292,11 +296,16 @@ function EditorCanvas( | |
|
||
return ( | ||
<BlockCanvas | ||
shouldIframe={ ! disableIframe } | ||
shouldIframe={ | ||
! disableIframe || [ 'Tablet', 'Mobile' ].includes( deviceType ) | ||
} | ||
contentRef={ contentRef } | ||
styles={ styles } | ||
height="100%" | ||
iframeProps={ iframeProps } | ||
iframeProps={ { | ||
...iframeProps, | ||
style: { ...iframeProps?.style, ...deviceStyles }, | ||
} } | ||
> | ||
{ themeSupportsLayout && | ||
! themeHasDisabledLayoutStyles && | ||
|
@@ -348,6 +357,7 @@ function EditorCanvas( | |
<BlockList | ||
className={ classnames( | ||
className, | ||
'is-' + deviceType.toLowerCase() + '-preview', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @youknowriad I got a ping from @sergiu-radu that we previous had this class on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can consider it adding it to that wrapper too. It would be good to open a detailed issue about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi guys 👋 No, this class was not on To be more specific, this class Now, in 6.5 the We will try to make some tests and adjust our code first but if we won't succeed - will open an issue. Thank you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be clear, per the extensibility guidelines of Gutenberg, this is an internal modifier class. It's unfortunate that theme authors are relying on it. We can try to mitigate the issues though (the class will probably end up being present in two places) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @youknowriad Why was it needed to move it from outside the canvas to inside the canvas? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR unified the behavior between post and site editors, which also meant some markup simplification (removing unnecessary wrappers in the post editor) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey guys, we made some more investigations and yes, the problem seems to not be related to that class actually but something else (most likely because of 2 scenarios - when we have iframe on desktop and when we don't have the iframe). Thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember exactly but I believe this class was not present at all in the site editor and I just kept it around (for BC) but in a unified place. We can consider moving it to anther place in. |
||
renderingMode !== 'post-only' | ||
? 'wp-site-blocks' | ||
: `${ blockListLayoutClass } wp-block-post-content` // Ensure root level blocks receive default/flow blockGap styling rules. | ||
|
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.
Don't we still need this? Although I see the floating toolbar even in trunk for the post editor...
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.
Yeah, it's a left over for a very old behavior.