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

refactor: clean up button-group styles #246

Merged
merged 1 commit into from May 1, 2022
Merged

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Apr 27, 2022

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 27, 2022
}

&:not(:last-child) {
border-bottom-right-radius: 0;
border-top-right-radius: 0;
}

&--active {
z-index: 1; /* Make the active button's border take precedence. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't make sense, don't know why it's needed at all.

@github-actions
Copy link

Dist CSS Diff

diff --git a/packages/core/dist/css/default/default.css b/packages/core/dist-branch/css/default/default.css
index 4266566..1e0eba0 100644
--- a/packages/core/dist/css/default/default.css
+++ b/packages/core/dist-branch/css/default/default.css
@@ -293,7 +293,7 @@
   --ifm-button-border-radius: calc(
     var(--ifm-global-radius) * var(--ifm-button-size-multiplier)
   );
-  --ifm-button-group-margin: 2px;
+  --ifm-button-group-spacing: 2px;
   --ifm-card-background-color: var(--ifm-background-surface-color);
   --ifm-card-border-radius: calc(var(--ifm-global-radius) * 2);
   --ifm-card-horizontal-spacing: var(--ifm-global-spacing);
@@ -2006,12 +2006,12 @@ hr {
 
 .button-group {
   display: inline-flex;
+  gap: var(--ifm-button-group-spacing);
 }
 
 .button-group > .button:not(:first-child) {
       border-bottom-left-radius: 0;
       border-top-left-radius: 0;
-      margin-left: var(--ifm-button-group-margin);
     }
 
 .button-group > .button:not(:last-child) {
@@ -2019,10 +2019,6 @@ hr {
       border-top-right-radius: 0;
     }
 
-.button-group > .button--active {
-      z-index: 1; /* Make the active button's border take precedence. */
-    }
-
 .button-group--block {
     display: flex;
     justify-content: stretch;

@github-actions
Copy link

Size Change: -684 B (0%)

Total Size: 569 kB

Filename Size Change
./packages/core/dist/css/default-dark/default-dark-rtl.css 82.6 kB -125 B (0%)
./packages/core/dist/css/default-dark/default-dark-rtl.min.css 58 kB -47 B (0%)
./packages/core/dist/css/default-dark/default-dark.css 82.5 kB -124 B (0%)
./packages/core/dist/css/default-dark/default-dark.min.css 58 kB -46 B (0%)
./packages/core/dist/css/default/default-rtl.css 80.5 kB -125 B (0%)
./packages/core/dist/css/default/default-rtl.min.css 56.9 kB -47 B (0%)
./packages/core/dist/css/default/default.css 80.5 kB -124 B (0%)
./packages/core/dist/css/default/default.min.css 56.8 kB -46 B (0%)
ℹ️ View Unchanged
Filename Size
./packages/core/dist/js/alerts.js 409 B
./packages/core/dist/js/alerts.min.js 409 B
./packages/core/dist/js/button-groups.js 267 B
./packages/core/dist/js/button-groups.min.js 267 B
./packages/core/dist/js/dropdowns.js 1.01 kB
./packages/core/dist/js/dropdowns.min.js 1.01 kB
./packages/core/dist/js/menu.js 2.4 kB
./packages/core/dist/js/menu.min.js 2.4 kB
./packages/core/dist/js/navbar.js 1.08 kB
./packages/core/dist/js/navbar.min.js 1.08 kB
./packages/core/dist/js/pills.js 270 B
./packages/core/dist/js/pills.min.js 270 B
./packages/core/dist/js/radio-behavior.js 705 B
./packages/core/dist/js/radio-behavior.min.js 705 B
./packages/core/dist/js/tabs.js 267 B
./packages/core/dist/js/tabs.min.js 267 B

compressed-size-action

@netlify
Copy link

netlify bot commented Apr 27, 2022

Deploy Preview for infima ready!

Name Link
🔨 Latest commit bce6aad
🔍 Latest deploy log https://app.netlify.com/sites/infima/deploys/6268c4a5abebcb0008e0eb50
😎 Deploy Preview https://deploy-preview-246--infima.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Support for gap seems inadequate: only 88%. Should we have some policies about what percentage we aim to support? (I also don't know if it can be mitigated through autoprefixing/etc. but from the dist CSS it seems unlikely)

@lex111
Copy link
Contributor Author

lex111 commented Apr 30, 2022

It's a pretty big support (eg margin-top has 94.5% support, not 100%), all major browsers support it, we already actively use this property in other places, so shouldn't be any problem here.

@Josh-Cena
Copy link
Contributor

OK let's do this 👍

margin-top not having 100% is because there's unknown support, though, not because it's actually unsupported

@Josh-Cena Josh-Cena merged commit 05fa7df into main May 1, 2022
@Josh-Cena Josh-Cena deleted the lex111/btn-group-optim branch May 1, 2022 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants