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

Site Logo has two width controls (one within Dimensions, when in a Row block) #47979

Open
richtabor opened this issue Feb 10, 2023 · 7 comments
Labels
[Block] Site Logo Affects the Site Logo Block [Feature] Layout Layout block support, its UI controls, and style output. [Type] Enhancement A suggestion for improvement.

Comments

@richtabor
Copy link
Member

richtabor commented Feb 10, 2023

Description

Currently the Dimensions > Width control renders if the Site Logo block is within a Row block.

Eventually it may be useful migrating width settings to Dimensions (though this particular control would need to always be present, not dependent on the Row block layout.  But in its current state, this control does not work for resizing the width of the uploaded image.

There's already a "Image width" control within the Site Logo block; we should rely only on that for the time being. Otherwise, there are competing controls, where only one works properly anyhow.

Note that this does not occur in Beta 1 — but with the Gutenberg trunk active.

Step-by-step reproduction instructions

  1. Add a Row block with a Site Logo inside
  2. See "Width" as a control within the block's Dimensions panel
  3. Change "Width" value, see error
  4. Add a Site Logo image to the block
  5. Change "Width" value, again see error
  6. Adjust the existing "Image width" RangeControl value, see changes properly applied

Screenshots, screen recording, code snippet

CleanShot.2023-02-10.at.14.00.51.mp4

Environment info

WP 6.2 Beta 1
Gutenberg trunk

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

@richtabor richtabor added [Block] Site Logo Affects the Site Logo Block [Feature] Layout Layout block support, its UI controls, and style output. labels Feb 10, 2023
@annezazu
Copy link
Contributor

I'm assuming you mean beta 1 for 6.2 rather than RC. Going to add this to the 6.2 board for consideration and want to alert @tellthemachines for thoughts here as I know you are doing a lot of work here!

@richtabor
Copy link
Member Author

I'm assuming you mean beta 1 for 6.2 rather than RC.

Yea, I've updated the issue.🤦‍♂️

For clarity, this doesn't happen in Beta 1, but does in Gutenberg.

@tellthemachines
Copy link
Contributor

For clarity, this doesn't happen in Beta 1, but does in Gutenberg.

That's weird, I can see it in 6.2 Beta 1 too. It definitely makes sense for it to be there given that #45364 made it in!

There's already a "Image width" control within the Site Logo block; we should rely only on that for the time being. Otherwise, there are competing controls, where only one works properly anyhow.

I think the problem here is that it might not be clear what "width" does for children of Row. It's not meant to resize the block itself, but redefine the space it occupies inside the Row block. So for instance, if we wanted to have Site Logo on one side and a couple other blocks at the other end of the Row, we could do it by setting Site Logo width to "fill":

Screenshot 2023-02-13 at 10 31 07 am

I'm not sure how we can make this clearer in the UI for those controls 😅 but I do think it's useful for all children of Row/Stack blocks (and ultimately for all children of flex layout blocks in general) to have them.

@richtabor
Copy link
Member Author

I think the problem here is that it might not be clear what "width" does for children of Row. It's not meant to resize the block itself, but redefine the space it occupies inside the Row block.

Ah, yea perhaps this clarification would help — cc @WordPress/gutenberg-design

@jameskoster
Copy link
Contributor

It may be tangential but I can't help wondering if we really need the Image Width setting. The overlap between it and Dimensions > Width is kind of confusing, both conceptually and practically. Personally I'd expect the image width to always respect whatever is set in Dimension > Width. Having both seems unnecessary?

if we wanted to have Site Logo on one side and a couple other blocks at the other end of the Row, we could do it by setting Site Logo width to "fill"

For this use case, wrapping the Site Logo in a container seems more natural than fiddling with two separate width controls. Maybe that's just me?

@richtabor
Copy link
Member Author

It may be tangential but I can't help wondering if we really need the Image Width setting. The overlap between it and Dimensions > Width is kind of confusing, both conceptually and practically.

Yea, we don't need Image width and Width. It would be nice if the "width" control essentially worked like "Image width" does.

@richtabor richtabor changed the title Site Logo should not have width dimension control if within a Row block Site Logo has two width controls (one within Dimensions, when in a Row block) Mar 22, 2023
@carolinan
Copy link
Contributor

Needing to wrap the block in a container to use width settings feels like it may be too advanced for some users?
It is already confusing for so many that you need a container to activate some alignment options.

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Site Logo Affects the Site Logo Block [Feature] Layout Layout block support, its UI controls, and style output. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants