Skip to content
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

Resizable Editor: Unintended vertical scrollbar #66297

Closed
2 tasks done
t-hamano opened this issue Oct 22, 2024 · 4 comments · Fixed by #66390
Closed
2 tasks done

Resizable Editor: Unintended vertical scrollbar #66297

t-hamano opened this issue Oct 22, 2024 · 4 comments · Fixed by #66390
Assignees
Labels
[Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@t-hamano
Copy link
Contributor

Description

In the resizable editor, if there is little content, the vertical scrollbar is not necessary, but it seems to always be displayed.

After investigating this issue, I found that it was caused by the top border added by #66041.

Step-by-step reproduction instructions

  • Open the site editor.
  • Open the pattern or template part editor.

Screenshots, screen recording, code snippet

Image

If I temporarily disable the top border this issue doesn't occur:

Image

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@t-hamano t-hamano added [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended labels Oct 22, 2024
@andrewserong
Copy link
Contributor

Oh, interesting, good catch!

It seems that the resize observer gives a rounded integer instead of the precise number of the actual size, and this border rule uses sub-pixels, which means we get a height of a resizable editor that is just not tall enough for the content:

const [ width, height ] = entrySize.map( ( d ) => Math.round( d ) );

(I believe we're using the legacy implementation because useResizeObserver is called without passing in a callback).

So, I'm not sure if this is the right way to fix it, but it seems like we could always add 1 to the height in the visual editor to ensure that the height accounts for the border. Would that be a terrible approach?

diff --git a/packages/editor/src/components/visual-editor/index.js b/packages/editor/src/components/visual-editor/index.js
index 8ad9ce7fc97..fe261fed4b7 100644
--- a/packages/editor/src/components/visual-editor/index.js
+++ b/packages/editor/src/components/visual-editor/index.js
@@ -380,15 +380,17 @@ function VisualEditor( {
 					'is-iframed': shouldIframe,
 				}
 			) }
 		>
 			<ResizableEditor
 				enableResizing={ enableResizing }
 				height={
-					sizes.height && ! forceFullHeight ? sizes.height : '100%'
+					sizes.height && ! forceFullHeight
+						? sizes.height + 1
+						: '100%'
 				}
 			>
 				<BlockCanvas
 					shouldIframe={ shouldIframe }
 					contentRef={ contentRef }
 					styles={ iframeStyles }
 					height="100%"

I can throw that up in a quick PR if it helps.

@andrewserong
Copy link
Contributor

Opened a PR in #66301 — happy to close it out, though, if there's a better fix!

@stokesman
Copy link
Contributor

Another day, another PR #66390 targeted to fix this.

@andrewserong
Copy link
Contributor

Another day, another PR #66390 targeted to fix this.

Thank you @stokesman, I appreciate your persistence here! 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
3 participants