Cover Block Height Input Conflicting with Min/Content-Height

[epiqueras]

Describe the Bug

The Cover Block's height input does not play nice with the block's min-height or content height.

To Reproduce

Steps to reproduce the behavior:

1. Set up a cover block with a solid background color or image.
2. Set its height to a value less than 50, click outside and see the value is unset.
3. Add some content and notice how you can set a height value smaller than the content height, but it just won't be respected.

Expected Behavior

The input should not let you input values lower than the content height or min-height.

Desktop (please complete the following information):

• OS: macOS Mojave 10.14.6 (18G95)
• Browser: Chrome
• Version: Version 77.0.3865.90 (Official Build) (64-bit)

Additional Context:

This was noted while reviewing #17371.

[senadir]

I believe this is the intended behavior that @jorgefilipecosta made in #17365

[epiqueras]

Yes, but:

Expected Behavior

The input should not let you input values lower than the content height or min-height.

[jorgefilipecosta]

The input should not let you input values lower than the content height or min-height.

Hi @epiqueras the input allows users to change the min-height, so although it seems the input is not being respected it is being respected as the min-height is being applied but as the content height is bigger the browser uses the content height.
Imagining a scenario where the content height is 700px, in one case you set the input height to 700px in another case you set to 500px both cases look the same originally. It seems there is no difference between setting a 500px or 700px height. Later a user removes some inner blocks from the cover in the case where the height was 700px the cover will still take 700px in the case where the height was 500px the cover height will decrease.
I think the only issue we have is that the field is called Height while it should be called Minimum Height.

[epiqueras]

@jorgefilipecosta

I think the only issue we have is that the field is called Height while it should be called Minimum Height.

Yes, and that you can't set a number that's less than 50 pixels:

Set its height to a value less than 50, click outside and see the value is unset.

[jorgefilipecosta]

Hi @epiqueras, I submitted a PR to correct the label #17634.

Yes, and that you can't set a number that's less than 50 pixels:

Set its height to a value less than 50, click outside and see the value is unset.

Regarding the problem, users can't set a value less than 50 pixels when the blur occurs if the value was less than 50px the input is ignored.
We can not make it impossible for a user to type the value "5" because the previous value may have been 580 and the user wants 590 so the user deletes the 0 and the 8 in order to then type 9 and 0, temporarily there was an input was value was "5" but we can not discard it right away when we can only discard it/ unset it when the blur occurs.

[epiqueras]

Regarding the problem, users can't set a value less than 50 pixels when the blur occurs if the value was less than 50px the input is ignored.

Why?

[jorgefilipecosta]

Why?

I'm not sure, but I think it was decided that the minimum height a user can set is 50px this minimum was already part of the logic since the first PR that implemented the resizing. It is a constraint setting a smaller value is not possible; to me, this constraint makes sense as a smaller value does not make much sense on the cover block.

[epiqueras]

Then we should keep the value and show a validation error message, "Please enter a value greater than...".

Just clearing it is confusing.

[mrwweb]

you can't set a number that's less than 50 pixels

This very much feels like a bug. It's a very poor user experience at best.

When I wanted to remove the minimum height just now I figured that entering 0 or 1 would effectively do that, but obviously not so. That my value is just silently discarded without any error or message felt extremely odd and bug-like, leading me here.

What I would propose is:

• Whatever the default minimum height is, that should be the default value of the field so users understand what that value is.
• Allow any value in the minimum height field (because why not?).

this constraint makes sense as a smaller value does not make much sense on the cover block.

That kind of makes sense, but seems like a dangerous assumption, and certainly my experience suggests one reason why someone would want a lower value.

If there is going to be a value constraint, then a bunch of UI work needs to be done to clearly communicate that. But just removing the constraint both is more expected and presumably easier.