-
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
Autoscaling/sizing Block previews #16113
Conversation
@jasmussen I'm working on an unreleased PR that handles some bugs in the preview scrolling or panel on general (see issue #16028), those styles will affect the overall structure of the element (a new div has to be created) also I'm switching the alignment of preview panel from absolute to using flex on the parent, I will try to test your changes with mine later this day and see how we can incorporate them both |
Worth noting that next steps for this PR likely includes adding an additional wrapper regardless, because we need 1 outer wrapper for container aspect ratio padding, and an inner wrapper that holds the content and is scaled down. So that might be fine. |
Alright, I've pushed a fair bit of polish to the component. The PR is now 95% complete. What's happened:
It works pretty well: The missing piece here is to refactor away the |
I tried applying your patch, but must've mangled it somehow, so I wasn't able to verify whether it worked as it should. Would definitely appreciate some code help. @gziolo my favorite :D do you have time for some feedback there? |
Thanks for looking at these. They are a central piece for the experience. |
1d1123d
to
85fc933
Compare
Alright friends, I think this is ready for review! The code that was added by @getdave (thank you and props) works great. I added a few tweaks, some cleanup, and a stub README. As it stands, this now works well. Remaining tasks are:
|
@jasmussen I'm struggling with how best to manually test this in browser. Might sound silly but do you have a nice workflow to test? |
For now I've simply just verified that every piece of the editor that uses this component looks right. That is, open the block library and hover over a reusable block, or open the switcher or styles panel for a block that has variations, such as Quote or Button. |
Can we address the pending comments and get this in? |
This reverts commit ea573f9. The reason being that there is no benefit of using shadow DOM and it introduces additional complexity.
After much experimenting we’ve decided that attempting to reliably detect the visual size of the Block’s visual contents is impossible. This is due to inconsistencies in the DOM markup of each Block. See #16113 (comment)
As discussed here #16113 (comment), there is little point in having thumbnail preview and large popover preview showing the same thing. Update to introduce a prop to opt in to scaling the preview. By default the preview is no auto-scaled. Apply `true` value to `BlockPreview` to ensure that Block Styles popover preview is scaled to fit.
0a22d9f
to
bdf16ad
Compare
No, any extra information from Matias. What I did were learning and experimenting with shadow DOM for a short time since I hadn't clue how it works. After reading and playing with this, conclude that although it could be a very powerful resource to handle components previsualization it's clearly out of the scope of this approach.
Being able to apply global and specific styles in a solid way to components. The idea behind using this approach is selecting specific styles to visualize each component. Generally, need to use global styles for all components (current one), but sometimes it could be useful to isolate specific styles for each component. Shadow DOM handles this case in an extremely safe way. Sorry by my mistake to not being 100% clear about experimenting around the shadow dom. Next time I'll create another branch. |
Seems like there is a unit test failing currently:
|
The direction of this has pivotted and is now running over at #16873 |
Closing in favor of #16873. |
Was previously using React createRef. Addresses #16113 (comment)
After much experimenting we’ve decided that attempting to reliably detect the visual size of the Block’s visual contents is impossible. This is due to inconsistencies in the DOM markup of each Block. See #16113 (comment)
As discussed here #16113 (comment), there is little point in having thumbnail preview and large popover preview showing the same thing. Update to introduce a prop to opt in to scaling the preview. By default the preview is no auto-scaled. Apply `true` value to `BlockPreview` to ensure that Block Styles popover preview is scaled to fit.
* First stab at a proof of concept. * Attempt to programmatically determine dest dimensions * Add README stub. * Updates to use default args for width and height Addresses #16113 (comment) * Update class namespaces to match package Addresses #16113 (comment) * Add width and height as Effect Hook deps Addresses #16113 (comment) * Use correct `createRef` from @wordpress/element Was previously using React createRef. Addresses #16113 (comment) * Revert "Update class namespaces to match package" This reverts commit 24252fa887986548839e7d9a6fe2639807ed0bc6. * Migrate to useRef hook * Adds basic unit test Still a WIP * Refactor to avoid unecessary state and simplify implementation of scaling * Add some padding to the preview Avoids Blocks butting up against the edges of the preview, especially if the preview wrapper has rounded corners which can cause the Blocks to be clipped. * Refactor to utilise JS template strings * WIP: Adds zoom for small individual blocks With small individual Blocks showing the preview at “actual size” (even when scaled to fit) is a bit odd. Everything still looks very zoomed out and far away. Updates to check DOM for largest element within the Block and use that width to calculate a “zoomed” scale value so that the contents of the Block itself is shown zoomed in. Still WIP * Updates to target block contents more reliably and to calc entire box model in width * block-preview: render scaled preview in a shadow dom * try: setting styles to shadow dom elements * Revert "try: setting styles to shadow dom elements" This reverts commit ea573f9. The reason being that there is no benefit of using shadow DOM and it introduces additional complexity. * Removes attempt at auto zoom on Block contents After much experimenting we’ve decided that attempting to reliably detect the visual size of the Block’s visual contents is impossible. This is due to inconsistencies in the DOM markup of each Block. See #16113 (comment) * Updates to remove scale by default in favour of opt in via prop As discussed here #16113 (comment), there is little point in having thumbnail preview and large popover preview showing the same thing. Update to introduce a prop to opt in to scaling the preview. By default the preview is no auto-scaled. Apply `true` value to `BlockPreview` to ensure that Block Styles popover preview is scaled to fit. * Adds scaleFactor prop to provide control for non dynamically scaled preview * Adjust scaleFactor for Block Styles preview * Fix to ensure hooks are called unconditionally to obey rules of Hooks See https://reactjs.org/docs/hooks-rules.html * Removes padding and border from preview on designer advice * Only apply dimensions to preview content when dynamically scaling * block-preview: a different approach attempt. * Correct typo in util function name * Seek the firstChild of the Block DOM node * Tidy up effect timers and improve comments * Dynamically calculate widths and offsets of preview container Removes hardcoded values. * Updates to account for widths of all Blocks passed into to preview Preivously we only considered a single Block scenario. However, the recent addition of multi block support for Previews means we need to ensure we’re measuring the widths of _all_ Blocks. * Revert class additions to Blocks * Adds optional scope to DOM utils. Use to limit scope in Block width comparison. * Update packages/block-editor/src/components/block-preview/index.js Co-Authored-By: Riad Benguella <benguella@gmail.com> * Allow prop based opt out from scaling * Fix to ensure smallest Block is found even if smaller than container Preivously we were default to the container width being the largest item. In fact we always want to determine the largest Block contents element and use that to determine the scale. * Remove unecessary Math.min usage * core/button: adjust dims of button wrapper * apply vertical alignment * set scale factor to 0.9 * rollback testing purpose comment * cpre/button: keep adjusting styles for preview * rename vars and css class names * adjust rebase * restore isScaled factor scale * set the scale adjustment in the proper place * update README file * core/button: tidy edit preview styles * coer/button: tweak button preview * preview-block: ensure make the preview visible. * block-preview: hide dropzone * core/button: adjust preview for thumb and full sizes * core/button: set same width for preview * core/button: because it's !important * core/button: set nowrap button in preview * core/button: set nowrap onlu for button * core/quote: adjust quote size * Make previews overflow to the bottom. Currently still has a bug where the large `.block-editor-block-switcher__preview` pane behaves like it's previewing a taller block, when it's not * Revert "core/button: set nowrap onlu for button" It breaks previews of long buttons This reverts commit 758468c. * Fix unit test * core/button: apply nowrap only to buttons * Try a fixed canvas width * Fix blocks editor styles * core/button: centering preview * core/button: adjust only into teh styles preview * block-preview: remove scaleAdjustment property * block-preview: hide inserte element in preview * block-preview: just pick up the first block to scale * block-preview: fix set tall class. X position (wip) * popover: remove commented lines * Refactor the preview * Remove debug code * simplify preview resets * Fix the preview recomputing * Vertical alignining small blocks * block-editor: restore viewportWidth as a property * Update Readme docs Props @marekhrabe. * Remove tests for now We'll revisit when we have a better idea what specifically we should be testing * block-preview: simplify comparision * Remove readme updates that break tests * Update docs properly. See 6f29c27
* First stab at a proof of concept. * Attempt to programmatically determine dest dimensions * Add README stub. * Updates to use default args for width and height Addresses #16113 (comment) * Update class namespaces to match package Addresses #16113 (comment) * Add width and height as Effect Hook deps Addresses #16113 (comment) * Use correct `createRef` from @wordpress/element Was previously using React createRef. Addresses #16113 (comment) * Revert "Update class namespaces to match package" This reverts commit 24252fa887986548839e7d9a6fe2639807ed0bc6. * Migrate to useRef hook * Adds basic unit test Still a WIP * Refactor to avoid unecessary state and simplify implementation of scaling * Add some padding to the preview Avoids Blocks butting up against the edges of the preview, especially if the preview wrapper has rounded corners which can cause the Blocks to be clipped. * Refactor to utilise JS template strings * WIP: Adds zoom for small individual blocks With small individual Blocks showing the preview at “actual size” (even when scaled to fit) is a bit odd. Everything still looks very zoomed out and far away. Updates to check DOM for largest element within the Block and use that width to calculate a “zoomed” scale value so that the contents of the Block itself is shown zoomed in. Still WIP * Updates to target block contents more reliably and to calc entire box model in width * block-preview: render scaled preview in a shadow dom * try: setting styles to shadow dom elements * Revert "try: setting styles to shadow dom elements" This reverts commit ea573f9. The reason being that there is no benefit of using shadow DOM and it introduces additional complexity. * Removes attempt at auto zoom on Block contents After much experimenting we’ve decided that attempting to reliably detect the visual size of the Block’s visual contents is impossible. This is due to inconsistencies in the DOM markup of each Block. See #16113 (comment) * Updates to remove scale by default in favour of opt in via prop As discussed here #16113 (comment), there is little point in having thumbnail preview and large popover preview showing the same thing. Update to introduce a prop to opt in to scaling the preview. By default the preview is no auto-scaled. Apply `true` value to `BlockPreview` to ensure that Block Styles popover preview is scaled to fit. * Adds scaleFactor prop to provide control for non dynamically scaled preview * Adjust scaleFactor for Block Styles preview * Fix to ensure hooks are called unconditionally to obey rules of Hooks See https://reactjs.org/docs/hooks-rules.html * Removes padding and border from preview on designer advice * Only apply dimensions to preview content when dynamically scaling * block-preview: a different approach attempt. * Correct typo in util function name * Seek the firstChild of the Block DOM node * Tidy up effect timers and improve comments * Dynamically calculate widths and offsets of preview container Removes hardcoded values. * Updates to account for widths of all Blocks passed into to preview Preivously we only considered a single Block scenario. However, the recent addition of multi block support for Previews means we need to ensure we’re measuring the widths of _all_ Blocks. * Revert class additions to Blocks * Adds optional scope to DOM utils. Use to limit scope in Block width comparison. * Update packages/block-editor/src/components/block-preview/index.js Co-Authored-By: Riad Benguella <benguella@gmail.com> * Allow prop based opt out from scaling * Fix to ensure smallest Block is found even if smaller than container Preivously we were default to the container width being the largest item. In fact we always want to determine the largest Block contents element and use that to determine the scale. * Remove unecessary Math.min usage * core/button: adjust dims of button wrapper * apply vertical alignment * set scale factor to 0.9 * rollback testing purpose comment * cpre/button: keep adjusting styles for preview * rename vars and css class names * adjust rebase * restore isScaled factor scale * set the scale adjustment in the proper place * update README file * core/button: tidy edit preview styles * coer/button: tweak button preview * preview-block: ensure make the preview visible. * block-preview: hide dropzone * core/button: adjust preview for thumb and full sizes * core/button: set same width for preview * core/button: because it's !important * core/button: set nowrap button in preview * core/button: set nowrap onlu for button * core/quote: adjust quote size * Make previews overflow to the bottom. Currently still has a bug where the large `.block-editor-block-switcher__preview` pane behaves like it's previewing a taller block, when it's not * Revert "core/button: set nowrap onlu for button" It breaks previews of long buttons This reverts commit 758468c. * Fix unit test * core/button: apply nowrap only to buttons * Try a fixed canvas width * Fix blocks editor styles * core/button: centering preview * core/button: adjust only into teh styles preview * block-preview: remove scaleAdjustment property * block-preview: hide inserte element in preview * block-preview: just pick up the first block to scale * block-preview: fix set tall class. X position (wip) * popover: remove commented lines * Refactor the preview * Remove debug code * simplify preview resets * Fix the preview recomputing * Vertical alignining small blocks * block-editor: restore viewportWidth as a property * Update Readme docs Props @marekhrabe. * Remove tests for now We'll revisit when we have a better idea what specifically we should be testing * block-preview: simplify comparision * Remove readme updates that break tests * Update docs properly. See 6f29c27
* First stab at a proof of concept. * Attempt to programmatically determine dest dimensions * Add README stub. * Updates to use default args for width and height Addresses #16113 (comment) * Update class namespaces to match package Addresses #16113 (comment) * Add width and height as Effect Hook deps Addresses #16113 (comment) * Use correct `createRef` from @wordpress/element Was previously using React createRef. Addresses #16113 (comment) * Revert "Update class namespaces to match package" This reverts commit 24252fa887986548839e7d9a6fe2639807ed0bc6. * Migrate to useRef hook * Adds basic unit test Still a WIP * Refactor to avoid unecessary state and simplify implementation of scaling * Add some padding to the preview Avoids Blocks butting up against the edges of the preview, especially if the preview wrapper has rounded corners which can cause the Blocks to be clipped. * Refactor to utilise JS template strings * WIP: Adds zoom for small individual blocks With small individual Blocks showing the preview at “actual size” (even when scaled to fit) is a bit odd. Everything still looks very zoomed out and far away. Updates to check DOM for largest element within the Block and use that width to calculate a “zoomed” scale value so that the contents of the Block itself is shown zoomed in. Still WIP * Updates to target block contents more reliably and to calc entire box model in width * block-preview: render scaled preview in a shadow dom * try: setting styles to shadow dom elements * Revert "try: setting styles to shadow dom elements" This reverts commit ea573f9. The reason being that there is no benefit of using shadow DOM and it introduces additional complexity. * Removes attempt at auto zoom on Block contents After much experimenting we’ve decided that attempting to reliably detect the visual size of the Block’s visual contents is impossible. This is due to inconsistencies in the DOM markup of each Block. See #16113 (comment) * Updates to remove scale by default in favour of opt in via prop As discussed here #16113 (comment), there is little point in having thumbnail preview and large popover preview showing the same thing. Update to introduce a prop to opt in to scaling the preview. By default the preview is no auto-scaled. Apply `true` value to `BlockPreview` to ensure that Block Styles popover preview is scaled to fit. * Adds scaleFactor prop to provide control for non dynamically scaled preview * Adjust scaleFactor for Block Styles preview * Fix to ensure hooks are called unconditionally to obey rules of Hooks See https://reactjs.org/docs/hooks-rules.html * Removes padding and border from preview on designer advice * Only apply dimensions to preview content when dynamically scaling * block-preview: a different approach attempt. * Correct typo in util function name * Seek the firstChild of the Block DOM node * Tidy up effect timers and improve comments * Dynamically calculate widths and offsets of preview container Removes hardcoded values. * Updates to account for widths of all Blocks passed into to preview Preivously we only considered a single Block scenario. However, the recent addition of multi block support for Previews means we need to ensure we’re measuring the widths of _all_ Blocks. * Revert class additions to Blocks * Adds optional scope to DOM utils. Use to limit scope in Block width comparison. * Update packages/block-editor/src/components/block-preview/index.js Co-Authored-By: Riad Benguella <benguella@gmail.com> * Allow prop based opt out from scaling * Fix to ensure smallest Block is found even if smaller than container Preivously we were default to the container width being the largest item. In fact we always want to determine the largest Block contents element and use that to determine the scale. * Remove unecessary Math.min usage * core/button: adjust dims of button wrapper * apply vertical alignment * set scale factor to 0.9 * rollback testing purpose comment * cpre/button: keep adjusting styles for preview * rename vars and css class names * adjust rebase * restore isScaled factor scale * set the scale adjustment in the proper place * update README file * core/button: tidy edit preview styles * coer/button: tweak button preview * preview-block: ensure make the preview visible. * block-preview: hide dropzone * core/button: adjust preview for thumb and full sizes * core/button: set same width for preview * core/button: because it's !important * core/button: set nowrap button in preview * core/button: set nowrap onlu for button * core/quote: adjust quote size * Make previews overflow to the bottom. Currently still has a bug where the large `.block-editor-block-switcher__preview` pane behaves like it's previewing a taller block, when it's not * Revert "core/button: set nowrap onlu for button" It breaks previews of long buttons This reverts commit 758468c. * Fix unit test * core/button: apply nowrap only to buttons * Try a fixed canvas width * Fix blocks editor styles * core/button: centering preview * core/button: adjust only into teh styles preview * block-preview: remove scaleAdjustment property * block-preview: hide inserte element in preview * block-preview: just pick up the first block to scale * block-preview: fix set tall class. X position (wip) * popover: remove commented lines * Refactor the preview * Remove debug code * simplify preview resets * Fix the preview recomputing * Vertical alignining small blocks * block-editor: restore viewportWidth as a property * Update Readme docs Props @marekhrabe. * Remove tests for now We'll revisit when we have a better idea what specifically we should be testing * block-preview: simplify comparision * Remove readme updates that break tests * Update docs properly. See 6f29c27
This PR is a work in progress. It starts work on improving the preview component to be way more useful. GIF:
The key thing that makes this work, is to calculate what the
scale
value should be, and the way to calculate that is relatively trivial.In fact, the component itself can be reauthored to receive a source width and a source height attribute, and it will take care of itself.
For example, if you want a landscape preview, you could do this:
and we could create JS to fill in the blanks. Specifically that math is outlined here:
https://github.com/WordPress/gutenberg/compare/try/better-previews?expand=1#diff-15792e4bd87a6bedb42355c3fd500733R67
Imagine the block you're previewing is an SVG. It has to have an explicit width and height, and then it can be scaled down perfectly according to the intrinsic aspect ratio that creates. In the code shown above, we use a 4:3 landscape aspect ratio, so the block is rendered at 400x300.
The next step is to calculate the scale value based on where the thumbnail will be shown. Right now the style variation previews are 124px wide, so the math looks like this:
Math.min( 124 / 400 )
, which comes to0.31
. And sure enough, when we applytransform: scale(0.31)
the preview looks perfect:The only missing piece here is that the container needs some responsive magic as well. It's here:
https://github.com/WordPress/gutenberg/compare/try/better-previews?expand=1#diff-15792e4bd87a6bedb42355c3fd500733R41
The height is 0, but a
padding-top: 75%
is applied. This padding provides the aspect ratio of the div to scale as a responsive container (see also this for more details). Where does that value come from? Simple:srcHeight / srcWidth * 100
. So for our case, it's300 / 400 * 100
=75
.So for example if you wanted a portrait 9:16 preview thumbnail, you might provide these dimensions:
This would provide a padding value of 177%. And if shown as a 200px wide thumbnail, the scale factor would be 0.25.
@youknowriad does the above make sense? If you have time feel free to take a look at the PR. Otherwise I'll explore improving the component as my next step. Right now the most difficult piece for me is that the
padding-top
has to be applied to the container of the preview, and the dimensions and scale factor has to be applied to the child of the container. So I might end up creating additional markup inside the preview component to make this happen.Terminology
The following screenshots helps to disambiguate some of the terms used to identify the different types of preview.
To Demo
Show Quote style before and after (big padding and large typography makes it overflow now)
Todo