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

WB-1635.fix - Fix border issue on Button.secondary #2124

Merged
merged 3 commits into from
Nov 17, 2023
Merged

Conversation

jandrade
Copy link
Member

@jandrade jandrade commented Nov 16, 2023

Summary:

In #2112, I introduced a new labelStyle prop and refactored the styles for the
secondary button. However, I missed a case where the border was not being
applied correctly, causing the button from being cut off in certain scenarios.
This PR fixes that issue by including the border in the resting/default state,
so the button is always rendered with a border (including that as part of
the box model).

NOTE: The issue was noticed while trying to integrate the changes in webapp.

Screenshot 2023-11-15 at 6 00 42 PM

Issue: WB-1635

Test plan:

Verify that the secondary button is rendered correctly in all scenarios. This
means that now secondary buttons will always have a border in the resting state.

@jandrade jandrade self-assigned this Nov 16, 2023
Copy link

changeset-bot bot commented Nov 16, 2023

🦋 Changeset detected

Latest commit: 4d6897b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@khanacademy/wonder-blocks-button Patch
@khanacademy/wonder-blocks-banner Patch
@khanacademy/wonder-blocks-birthday-picker Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@khan-actions-bot khan-actions-bot requested a review from a team November 16, 2023 23:12
@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Nov 16, 2023

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/red-maps-talk.md, packages/wonder-blocks-button/src/components/button-core.tsx, packages/wonder-blocks-button/src/__tests__/__snapshots__/custom-snapshot.test.tsx.snap

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

Copy link
Contributor

github-actions bot commented Nov 16, 2023

Size Change: +13 B (0%)

Total Size: 92.7 kB

Filename Size Change
packages/wonder-blocks-button/dist/es/index.js 4.27 kB +13 B (0%)
ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.67 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.72 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 1.13 kB
packages/wonder-blocks-cell/dist/es/index.js 2.19 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.24 kB
packages/wonder-blocks-color/dist/es/index.js 1.15 kB
packages/wonder-blocks-core/dist/es/index.js 3.67 kB
packages/wonder-blocks-data/dist/es/index.js 6.33 kB
packages/wonder-blocks-dropdown/dist/es/index.js 12 kB
packages/wonder-blocks-form/dist/es/index.js 5.34 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-i18n/dist/es/index.js 4.54 kB
packages/wonder-blocks-icon-button/dist/es/index.js 3.06 kB
packages/wonder-blocks-icon/dist/es/index.js 3.04 kB
packages/wonder-blocks-labeled-field/dist/es/index.js 72 B
packages/wonder-blocks-layout/dist/es/index.js 1.88 kB
packages/wonder-blocks-link/dist/es/index.js 2.54 kB
packages/wonder-blocks-modal/dist/es/index.js 5.04 kB
packages/wonder-blocks-pill/dist/es/index.js 1.03 kB
packages/wonder-blocks-popover/dist/es/index.js 4.33 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.51 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.55 kB
packages/wonder-blocks-spacing/dist/es/index.js 158 B
packages/wonder-blocks-switch/dist/es/index.js 2.06 kB
packages/wonder-blocks-testing/dist/es/index.js 3.94 kB
packages/wonder-blocks-theming/dist/es/index.js 1.21 kB
packages/wonder-blocks-timing/dist/es/index.js 1.78 kB
packages/wonder-blocks-toolbar/dist/es/index.js 862 B
packages/wonder-blocks-tooltip/dist/es/index.js 5.05 kB
packages/wonder-blocks-typography/dist/es/index.js 1.49 kB

compressed-size-action

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Merging #2124 (4d6897b) into main (834abb9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2124   +/-   ##
=======================================
  Coverage   97.07%   97.07%           
=======================================
  Files         243      243           
  Lines       28247    28250    +3     
  Branches     2466     2420   -46     
=======================================
+ Hits        27420    27423    +3     
  Misses        827      827           
Files Coverage Δ
...onder-blocks-button/src/components/button-core.tsx 99.80% <100.00%> (+<0.01%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 834abb9...4d6897b. Read the comment docs.

Copy link
Contributor

github-actions bot commented Nov 16, 2023

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (dce29ba) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2124"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2124

Copy link
Contributor

github-actions bot commented Nov 16, 2023

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-cfrdbjdkjg.chromatic.com/

Chromatic results:

Metric Total
Captured snapshots 0
Tests with visual changes 13
Total stories 393
Inherited (not captured) snapshots [TurboSnap] 321
Tests on the build 321

@jandrade jandrade merged commit 01593e2 into main Nov 17, 2023
13 checks passed
@jandrade jandrade deleted the button-outline-fix branch November 17, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants