-
Notifications
You must be signed in to change notification settings - Fork 77
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(split-button): Make dividers consistent #9298
feat(split-button): Make dividers consistent #9298
Conversation
@@ -98,6 +98,7 @@ | |||
:host([appearance="outline"]) { | |||
.divider-container { | |||
border-block: 1px solid var(--calcite-split-button-divider); | |||
background-color: initial; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The divider container's background-color is set via custom CSS prop, so could you try updating --calcite-split-button-background
instead of overriding here? It looks like it should be transparent
for both outline
and outline-fill
appearance types.
… disabled button;
…github.com:Esri/calcite-design-system into josercarcamo/8142-split-button-consistent-dividers
@@ -64,16 +64,19 @@ | |||
:host([appearance="outline-fill"]) { | |||
&:host([kind="brand"]) { | |||
--calcite-split-button-divider: theme("colors.brand"); | |||
--calcite-split-button-background: transparent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the same value is applied regardless of kind
, can you look at moving this declaration one level up?
:host([appearance="outline"]),
:host([appearance="outline-fill"]) {
--calcite-split-button-background: transparent;
&:host[kind="brand"]) {
--calcite-split-button-divider: theme("colors.brand");
}
/* ... */
Also, this block overrides these styles, so we can probably remove the overridden ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of specificity, I can't factor out this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. Disregard my comment then. Same with the disabled styling one below.
Sidebar: I think we could refactor this to simplify styles.
packages/calcite-components/src/components/split-button/split-button.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Related Issue: #8142
Summary
Made the dividers look consistent for all split button appearances.