Skip to content

Fix: Add the border width on initial color change for border - #69112

Open
hbhalodia wants to merge 7 commits into
WordPress:trunkfrom
hbhalodia:fix/issue-69094
Open

hbhalodia wants to merge 7 commits into
WordPress:trunkfrom
hbhalodia:fix/issue-69094

Conversation

@hbhalodia

@hbhalodia hbhalodia commented Feb 10, 2025 •

Copy link
Copy Markdown
Contributor

What?

Fixes: #69094

  • PR adds the condition of undefined on initial render to set the width to 1px for border, if user adds/changes the color before explicitly setting the width.

Why?

How?

  • PR adds the undefined condition with the 0 width check.

Testing Instructions

  1. Open any page/post.
  2. Add any paragraph.
  3. Go to inspector control and enable border.
  4. Set the border color first, you would notice it would also add 1px width for border.

Testing Instructions for Keyboard

  • None

Screenshots or screencast

BorderFix.mov

@hbhalodia hbhalodia self-assigned this Feb 10, 2025
@hbhalodia
hbhalodia marked this pull request as ready for review February 10, 2025 05:29
@hbhalodia
hbhalodia requested a review from ajitbohra as a code owner February 10, 2025 05:29
@github-actions

github-actions Bot commented Feb 10, 2025 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: coder-rancho <vyasnaman@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: viralsampat-multidots <mdviralsampat@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Comment thread packages/components/src/border-control/border-control-dropdown/hook.ts Outdated
@t-hamano t-hamano added [Package] Components /packages/components [Type] Bug An existing feature does not function as intended labels Feb 20, 2025
@hbhalodia
hbhalodia requested a review from a team as a code owner August 24, 2026 12:11
@hbhalodia
hbhalodia requested review from Mamaduka and t-hamano August 24, 2026 12:11

@mirka mirka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like theme.json supports any string as border width (e.g. thin, var()), and that gets sent down to this component. The component can't render those values correctly in that case, but regardless, is it a problem that we silently overwrite these intentional string values with 1px? (cc @aaronrobertshaw)

Comment thread packages/components/CHANGELOG.md Outdated
- `FontSizePicker`: Remove Custom option from dropdown to prevent unexpected context changes during keyboard navigation ([#69038](https://github.com/WordPress/gutenberg/pull/69038)).

- `ComboboxControl`: Add an `isLoading` prop to show a loading spinner ([#68990](https://github.com/WordPress/gutenberg/pull/68990))
- `BorderControl`: adjust `useBorderControlDropdown` hook to use `1px` width when selecting the border color initially without explicitly setting width ([#69112](https://github.com/WordPress/gutenberg/pull/69112)).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changelog needs to be repositioned.

Also, I don't think we need to mention useBorderControlDropdown specifically, since that's internal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mirka, This is now updated.

@t-hamano

t-hamano commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Blocks may already have borders applied via global styles or custom CSS. In such cases, simultaneously adding a 1px width along with a color change might cause confusion.

border.mp4

It looks like theme.json supports any string as border width (e.g. thin, var()), and that gets sent down to this component. The component can't render those values correctly in that case, but regardless, is it a problem that we silently overwrite these intentional string values with 1px? (cc @aaronrobertshaw)

There was a similar discussion in the past regarding whether one border property should be changed simultaneously with another. #75431 (comment)

Considering that opinion, it might be better not to add the 1px width. cc @aaronrobertshaw @ramonjd @andrewserong @talldan

@ramonjd

ramonjd commented Aug 26, 2026

Copy link
Copy Markdown
Member

Thanks for the ping!

In my testing, this PR introduces something similar to what was discussed over in #75431 (comment)

There, the change discards the user's explicit color choice from global styles.

Here, it's discarding the explicit width:

Kapture.2026-08-26.at.11.45.26.mp4

I think the idea that he user should be able to view the border immediately when setting a color and not a width is a worthy one to think about, but I don't think we should cause unexpected changes.

A theme that defines a border width presumably wants to enforce that unless an editor explicitly changes it via the control, not via the side-effect of another control.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gutenberg border block setting should be working properly.

5 participants