Skip to content

Conversation

@stokesman
Copy link
Contributor

@stokesman stokesman commented Sep 18, 2020

In order to fix #18906, this PR changes the minimum height of the spacer block to 1px and introduces a minimum height of 48px 20px in the editor to ensure ease of block selection.

spacer-min-20px

EDIT: Reduced the minimum height to 20 pixels in the editor in accordance with the current minimum height and removed some margin that was a solution for inserter overlap that is tangential to this PR.

How has this been tested?

Wordpress v5.5.1 with Gutenberg plugin v9.0.0

Types of changes

Nonbreakers

Checklist:

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

@talldan talldan added [Block] Spacer Affects the Spacer Block [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. labels Sep 21, 2020
@talldan talldan requested a review from mapk September 21, 2020 02:19
@talldan
Copy link
Contributor

talldan commented Sep 21, 2020

@mapk I hope you don't mind, but I added you for review as I know you were involved in the original issue.

This approach seems like it might be a reasonable trade-off. The downside that I pointed out in the issue is that the spacer size in the editor is no longer consistent with the post itself at sizes less than 20px because the block always has that minimum height in the editor.

@talldan talldan added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 21, 2020
@mapk
Copy link
Contributor

mapk commented Sep 21, 2020

The downside that I pointed out in the issue is that the spacer size in the editor is no longer consistent with the post itself at sizes less than 20px because the block always has that minimum height in the editor.

I agree here. While I understand some things just aren't going to be perfectly rendered in the Editor as they are on the front end, this particular block should. It is used to increase visual padding between elements and should reflect that padding accurately, IMO.

I really dig the exploration here, but I think we need to keep iterating.

@stokesman stokesman force-pushed the try/spacer-edit-min-height branch from 67094f0 to 1d4307f Compare September 22, 2020 02:47
@talldan
Copy link
Contributor

talldan commented Sep 24, 2020

Closing as superseded by #25528, thanks for the exploration on this issue. ❤️

@talldan talldan closed this Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Spacer Affects the Spacer Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository 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.

Spacer Block: cannot adjust height to less than 20px

3 participants