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

feat(close-button): remove close button #931

Merged
merged 4 commits into from
Apr 8, 2022
Merged

Conversation

dierat
Copy link
Contributor

@dierat dierat commented Apr 1, 2022

Summary:

BREAKING CHANGE:

The CloseButton is kind of a snowflake because it has a hover effect for the icon button that we don't use anywhere else. I talked to Sean and we're good to remove it to make the close buttons consistent with our other icon buttons.

This PR removes the component entirely. These are the PRs removing its callsites (they've been approved and merged):

Test Plan:

All the tests pass, and there's no reference to this CloseButton in traject except for the flow types.

@dierat
Copy link
Contributor Author

dierat commented Apr 1, 2022

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #187521: Remove CloseButton component.

Base automatically changed from diedra/toast/plain-close to main April 3, 2022 03:52
@codecov
Copy link

codecov bot commented Apr 3, 2022

Codecov Report

Merging #931 (e5bab8b) into main (f1fee2f) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #931      +/-   ##
==========================================
- Coverage   95.86%   95.78%   -0.09%     
==========================================
  Files         142      140       -2     
  Lines        1162     1138      -24     
  Branches      181      173       -8     
==========================================
- Hits         1114     1090      -24     
  Misses         47       47              
  Partials        1        1              
Impacted Files Coverage Δ
src/CloseButton/CloseButton.tsx
src/CloseButton/CloseButton.stories.tsx

Continue to review full report at Codecov.

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

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

size-limit report

Path Size
components 71.66 KB (-1.92% 🔽)
styles 4.49 KB (0%)

@dierat dierat marked this pull request as ready for review April 5, 2022 15:06
@dierat dierat requested a review from a team April 5, 2022 15:07
@dierat
Copy link
Contributor Author

dierat commented Apr 5, 2022

Copy link
Contributor

@anniehu4 anniehu4 left a comment

Choose a reason for hiding this comment

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

👋

@dierat dierat merged commit c3c2151 into main Apr 8, 2022
@dierat dierat deleted the diedra/close-button/remove branch April 8, 2022 20:40
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.

2 participants