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

Adding a maximum height to the preview of the long blocks #24493

Merged
merged 3 commits into from
Aug 26, 2020

Conversation

aktasfatih
Copy link
Member

@aktasfatih aktasfatih commented Aug 11, 2020

Description

Closes #24484

  • The preview of large blocks exceeds the whole page.

How has this been tested?

  • I created a table block with 200 rows and checked the preview.

Types of changes

  • I added a maximum height of 700px to the preview of the blocks
  • Fix: It now uses the height of the inserter minus the top position.
  • The overflowing part of the tall block is hidden.

download

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Added a maximum height for the preview of large blocks
…r minus the top position of the preview window
@aktasfatih aktasfatih changed the title Issue: #24484 Adding a maximum height to the preview of the long blocks Aug 11, 2020
@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. labels Aug 12, 2020
@senadir
Copy link
Contributor

senadir commented Aug 12, 2020

While this fixes the inserter, style switcher is still broken, I suggest you use max-height

@aktasfatih
Copy link
Member Author

What should be the maximum height for the preview of the styles? I chose it to be 160px, which is a double of its minimum height of 80px. Would that be enough?

@aktasfatih
Copy link
Member Author

This is what it looks like. I expanded the height of the separator line just to test the styles. It seems to successfully cut the long block.
save

@MichaelArestad
Copy link
Contributor

@aktasfatih the design of the style previews is likely going to change. I think for now a max height of 160px is okay for now, though I'm not sure it's necessary for this PR.

@aktasfatih
Copy link
Member Author

Should I remove the change for style previews or keep it? And what do you think about the preview of the main blocks? Thanks!

@MichaelArestad
Copy link
Contributor

Should I remove the change for style previews or keep it?

I am unsure if it's necessary yet. I haven't encountered long blocks with previews so it's hard to say

And what do you think about the preview of the main blocks? Thanks!

The main block preview looks ace! Nice work!

@youknowriad
Copy link
Contributor

The first screenshot in #24484 shows an undesirable effect for the styles preview. So if this ok design-wise, I think we should keep the min height.

@senadir What do you think here? Does it address your issues?

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@youknowriad youknowriad merged commit 7ee1c04 into WordPress:master Aug 26, 2020
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit block preview height.
4 participants