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

BorderControl: Use __next40pxDefaultSize prop for Reset button #65682

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Sep 26, 2024

What?

This PR updates the "Reset" button of the BorderControl component to use the new 40px default size.

Why?

Discovered as I was auditing button sizes as part of reviewing #65018.

How?

We're adding the __next40pxDefaultSize prop to the "Reset" button. The button was already set to 40px through CSS, but we no longer need that style override when we can just opt in with the __next40pxDefaultSize prop.

Testing Instructions

  • Open Storybook: /?path=/story/components-experimental-bordercontrol--default
  • Pick a color.
  • Verify the "Reset" button is still the right size (40px).

Testing Instructions for Keyboard

Same

Screenshots or screencast

Screenshot 2024-09-26 at 18 24 28

@tyxla tyxla added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Sep 26, 2024
@tyxla tyxla requested a review from a team September 26, 2024 15:25
@tyxla tyxla self-assigned this Sep 26, 2024
Copy link

github-actions bot commented Sep 26, 2024

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: tyxla <[email protected]>
Co-authored-by: ciampo <[email protected]>

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

@tyxla tyxla force-pushed the update/border-control-reset-size-prop branch from cabdda2 to 029cbb5 Compare September 26, 2024 15:27
Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

🚀

@tyxla tyxla merged commit 1cfb070 into trunk Sep 26, 2024
62 checks passed
@tyxla tyxla deleted the update/border-control-reset-size-prop branch September 26, 2024 16:28
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 26, 2024
@mirka mirka added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 1, 2024
Copy link

github-actions bot commented Oct 1, 2024

There was a conflict while trying to cherry-pick the commit to the wp/6.7 branch. Please resolve the conflict manually and create a PR to the wp/6.7 branch.

PRs to wp/6.7 are similar to PRs to trunk, but you should base your PR on the wp/6.7 branch instead of trunk.

# Checkout the wp/6.7 branch instead of trunk.
git checkout wp/6.7
# Create a new branch for your PR.
git checkout -b my-branch
# Cherry-pick the commit.
git cherry-pick 1cfb0707a0bb4a99d7b53450bd0cf51077db9af3
# Check which files have conflicts.
git status
# Resolve the conflict...
# Add the resolved files to the staging area.
git status
git add .
git cherry-pick --continue
# Push the branch to the repository
git push origin my-branch
# Create a PR and set the base to the wp/6.7 branch.
# See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request.

@ciampo
Copy link
Contributor

ciampo commented Oct 1, 2024

⬆️ This usually happens when a dependant PR was not cherry-picked . I managed to solve a similar issue by cherry picking the dependendant PR, and then removing+re-adding the cherry-pick label to the current PR

@mirka
Copy link
Member

mirka commented Oct 1, 2024

@ciampo Thanks for the hint. Aaandd ugh it's a changelog conflict.

@mirka mirka removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 1, 2024
@ciampo
Copy link
Contributor

ciampo commented Oct 1, 2024

Aaandd ugh it's a changelog conflict.

Yeah, improving the CHANGELOG is another thing that is never high priority enough but keeps being annoying

@tyxla
Copy link
Member Author

tyxla commented Oct 7, 2024

If I understand correctly, this has been backported already, right?

@mirka
Copy link
Member

mirka commented Oct 7, 2024

If I understand correctly, this has been backported already, right?

No, I rescinded it from backport because the changelog stuff was too annoying and the formal size deprecation was punted to 6.8 😅

@tyxla
Copy link
Member Author

tyxla commented Oct 7, 2024

Ah, got it. Thanks for confirming @mirka 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants