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

Should "Border & Shadow" panel label include "border" if border is not supported? #58907

Closed
carolinan opened this issue Feb 10, 2024 · 9 comments · Fixed by #59358
Closed

Should "Border & Shadow" panel label include "border" if border is not supported? #58907

carolinan opened this issue Feb 10, 2024 · 9 comments · Fixed by #59358
Assignees
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@carolinan
Copy link
Contributor

carolinan commented Feb 10, 2024

Description

Not all themes support borders, so it can be confusing when a panel labeled Border and Shadow has no border controls.
I'm not sure if this is a bug, it felt unexpected to me, but I understand that consistency on the panel labels is also important.

Step-by-step reproduction instructions

On WordPress 6.5 alpha:
Activate a classic theme like Twenty Sixteen
In the block editor, add an image block.
Open the block settings panel, locate the "Border and Shadow" panel.
Now add a button:
Open the block settings panel, locate the "Border and Shadow" panel.

Compare the two experiences.

Screenshots, screen recording, code snippet

Border and shadow panel without border controls

Environment info

WordPress 6.5-alpha-57580

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

@carolinan carolinan added the [Type] Bug An existing feature does not function as intended label Feb 10, 2024
@jordesign jordesign added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Feb 11, 2024
@annezazu
Copy link
Contributor

cc @WordPress/gutenberg-design for thoughts here. I agree this is confusing!

@colorful-tones
Copy link
Member

I believe that PR #58776 will address this issue, but waiting for confirmation (see comment).

@jameskoster
Copy link
Contributor

I agree the scenario described is confusing.

Adding logic to adjust panel titles doesn't seem great though... I guess this raises the question about whether Border and Shadow (Effects) should be separate panels, but that's another conversation.

@colorful-tones colorful-tones added the [Status] In Progress Tracking issues with work in progress label Feb 13, 2024
@jasmussen
Copy link
Contributor

Adding logic to adjust panel titles doesn't seem great though... I guess this raises the question about whether Border and Shadow (Effects) should be separate panels, but that's another conversation.

Border & shadow feel connected to me, enough that they belong together. What's your main concern with adjusting panel titles? I feel that's the best path forward.

@t-hamano
Copy link
Contributor

t-hamano commented Feb 18, 2024

How about changing it to a more abstract title like "Outline" or "Frame" or "Contour" like the other panels?

@jameskoster
Copy link
Contributor

What's your main concern with adjusting panel titles?

Mostly the resulting inconsistencies. I feel panel headings should be immutable for immediate recognition across contexts. Not to mention documentation. Making one heading dynamic sets a precedent that they can all be dynamic.

I don't want to derail the conversation, but I'd lean towards something like the Figma approach; a dedicated panel for border, then something more abstract like "Effects" for lesser-used features like shadows and filters.

@jasmussen
Copy link
Contributor

I hear you, to me it's no different than a panel being contextual to the block being selected. You could think of it as three panels if you like, border, border & shadow, and shadow. I know that doesn't address your concern. But the benefit of having a single panel, given the behavior and current design of panels that we have, IMO vastly outweigh the benefits of separate panels. If we had the same level of density as Figma does, things would be different. And they can be in the future, too, if we explore improvements to the inspector and panels, so I'd earmark that as something to consider. But until we have more density, it would just be more visual noise, more tab-stops, and dubious user benefit.

@SaxonF
Copy link
Contributor

SaxonF commented Feb 20, 2024

I guess this raises the question about whether Border and Shadow (Effects) should be separate panels, but that's another conversation.

I would +1 this

@madhusudhand
Copy link
Member

I hear you, to me it's no different than a panel being contextual to the block being selected. You could think of it as three panels if you like, border, border & shadow, and shadow. I know that doesn't address your concern. But the benefit of having a single panel, given the behavior and current design of panels that we have, IMO vastly outweigh the benefits of separate panels.

Created #59358 where it sets the panel label based on the controls enabled. I guess we can move forward with this approach for 6.5 and keep this issue open for discussion on other ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

9 participants