Skip to content

fix(button): support for wrapping text #2248

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 20, 2024
Merged

fix(button): support for wrapping text #2248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
869c246
fix(button): restore ability for button text to wrap
jawinn Oct 27, 2023
7d73a9c
docs(button): add wrapping text story
jawinn Oct 30, 2023
b828cea
fix(button)!: icon alignment when text is wrapping
jawinn Oct 31, 2023
c80678b
docs(button): rename storybook control to layout
jawinn Nov 16, 2023
0fe8bdc
fix(button): support any existing use of ui icons
jawinn Feb 7, 2024
c81a6f2
feat(button): mod property for icon margin top
jawinn Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 45 additions & 20 deletions components/button/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ governing permissions and limitations under the License.
--spectrum-button-focus-ring-border-radius: calc(var(--spectrum-button-border-radius) + var(--spectrum-button-focus-ring-gap));
--spectrum-button-focus-ring-thickness: var(--spectrum-focus-indicator-thickness);
--spectrum-button-focus-indicator-color: var(--spectrum-focus-indicator-color);
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-50);
}


.spectrum-Button--sizeS {
--spectrum-button-min-width: calc(var(--spectrum-component-height-75) * var(--spectrum-button-minimum-width-multiplier));

Expand All @@ -40,6 +40,8 @@ governing permissions and limitations under the License.
--spectrum-button-padding-label-to-icon: var(--spectrum-text-to-visual-75);
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-small);
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-small);
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-75);
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-75);
}

.spectrum-Button--sizeM {
Expand All @@ -56,6 +58,8 @@ governing permissions and limitations under the License.
--spectrum-button-padding-label-to-icon: var(--spectrum-text-to-visual-100);
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-medium);
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-medium);
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-100);
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-100);

&[pending],
&.is-pending {
Expand All @@ -77,7 +81,8 @@ governing permissions and limitations under the License.
--spectrum-button-padding-label-to-icon: var(--spectrum-text-to-visual-200);
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-large);
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-large);

--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-200);
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-200);

&[pending],
&.is-pending {
Expand All @@ -99,6 +104,8 @@ governing permissions and limitations under the License.
--spectrum-button-padding-label-to-icon: var(--spectrum-text-to-visual-300);
--spectrum-button-top-to-text: var(--spectrum-button-top-to-text-extra-large);
--spectrum-button-bottom-to-text: var(--spectrum-button-bottom-to-text-extra-large);
--spectrum-button-top-to-icon: var(--spectrum-component-top-to-workflow-icon-300);
--spectrum-button-intended-icon-size: var(--spectrum-workflow-icon-size-300);

.spectrum--medium &[pending],
.spectrum--medium &.is-pending {
Expand Down Expand Up @@ -141,11 +148,28 @@ governing permissions and limitations under the License.
}

.spectrum-Icon {
/* Any block-size difference between the intended workflow icon size and actual icon used.
Helps support any existing use of smaller UI icons instead of intended Workflow icons. */
--_icon-size-difference: max(0px,
var(--spectrum-button-intended-icon-size) -
var(--spectrum-icon-block-size, var(--spectrum-button-intended-icon-size))
);

margin-block-start: var(--mod-button-icon-margin-block-start,
max(0px,
var(--mod-button-top-to-icon, var(--spectrum-button-top-to-icon)) -
var(--mod-button-border-width, var(--spectrum-button-border-width)) +
(var(--_icon-size-difference, 0px) / 2)
)
);

margin-inline-start: calc(
var(--mod-button-edge-to-visual, var(--spectrum-button-edge-to-visual)) -
var(--mod-button-edge-to-text, var(--spectrum-button-edge-to-text))
);
color: inherit;
flex-shrink: 0;
align-self: flex-start;
}

/* correct focus indicator radius for t-shirt sizing */
Expand All @@ -159,7 +183,9 @@ governing permissions and limitations under the License.
border-radius: 50%;

.spectrum-Icon {
align-self: center;
margin-inline-start: 0;
margin-block-start: 0;
}

&::after {
Expand All @@ -176,9 +202,13 @@ a.spectrum-Button {
@extend %spectrum-ButtonLabel;
padding-block-start: calc(var(--mod-button-top-to-text, var(--spectrum-button-top-to-text)) - var(--mod-button-border-width, var(--spectrum-button-border-width)));
padding-block-end: calc(var(--mod-button-bottom-to-text, var(--spectrum-button-bottom-to-text)) - var(--mod-button-border-width, var(--spectrum-button-border-width)));
white-space: nowrap;
line-height: var(--mod-button-line-height, var(--spectrum-button-line-height));
align-self: start;
text-align: var(--mod-button-text-align, center);
}

.spectrum-Button .spectrum-Icon + .spectrum-Button-label {
text-align: var(--mod-button-text-align-with-icon, start);
}

.spectrum-Button {
Expand All @@ -190,26 +220,22 @@ a.spectrum-Button {
}
}

/* special cases for focus-ring */
/* Special cases for focus indicator */
.spectrum-Button {
transition: border-color var(--mod-button-animation-duration, var(--spectrum-button-animation-duration)) ease-in-out;

&::after {
position: absolute;
inset: 0;

margin: calc((var(--mod-button-focus-ring-gap, var(--spectrum-button-focus-ring-gap)) + var(--mod-button-border-width, var(--spectrum-button-border-width))) * -1);

border-radius: var(--mod-button-focus-ring-border-radius, var(--spectrum-button-focus-ring-border-radius));

transition: box-shadow var(--mod-button-animation-duration, var(--spectrum-button-animation-duration)) ease-in-out;

pointer-events: none;
content: '';
}

&:focus-visible {
/* kill the default ring */
/* Remove the default focus outline */
box-shadow: none;
outline: none;

Expand All @@ -222,7 +248,6 @@ a.spectrum-Button {

/* Core Token Theming */
/* former skin.css, applied / copied from actionbutton/index.css */

.spectrum-Button {
@extend %spectrum-BaseButton;

Expand Down Expand Up @@ -268,7 +293,16 @@ a.spectrum-Button {
}
}

/* windows high contrast mode over-writes for accent variant */
/* Static color variants */
.spectrum-Button--staticWhite {
--spectrum-button-focus-indicator-color: var(--mod-static-black-focus-indicator-color, var(--spectrum-static-black-focus-indicator-color));
}

.spectrum-Button--staticBlack {
--spectrum-button-focus-indicator-color: var(--mod-static-black-focus-indicator-color, var(--spectrum-static-black-focus-indicator-color));
}
Comment on lines +297 to +303
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these 2 need increased specificity by incorporating the .spectrum-Button selector as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, unless there's a scenario you're thinking of? I just moved these up a tiny bit above the high contrast styles. They are still at the end of the file and override the same specificity of their definitions in .spectrum-Button.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't rely on order for apply styles appropriately because a lot of processing moves things around. Just raising the question because I've seen a few components where insufficient specificity was causing styles not to be applied after processing.


/* Windows High Contrast Mode */
@media (forced-colors: active) {
.spectrum-Button {
--highcontrast-button-content-color-disabled: GrayText;
Expand Down Expand Up @@ -302,12 +336,3 @@ a.spectrum-Button {
}
}
}

/* static variants */
.spectrum-Button--staticWhite {
--spectrum-button-focus-indicator-color: var(--mod-static-black-focus-indicator-color, var(--spectrum-static-black-focus-indicator-color));
}

.spectrum-Button--staticBlack {
--spectrum-button-focus-indicator-color: var(--mod-static-black-focus-indicator-color, var(--spectrum-static-black-focus-indicator-color));
}
4 changes: 4 additions & 0 deletions components/button/metadata/mods.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,17 @@
| `--mod-button-font-family` |
| `--mod-button-font-size` |
| `--mod-button-height` |
| `--mod-button-icon-margin-block-start` |
| `--mod-button-line-height` |
| `--mod-button-margin-block` |
| `--mod-button-margin-left` |
| `--mod-button-margin-right` |
| `--mod-button-min-width` |
| `--mod-button-padding-label-to-icon` |
| `--mod-button-static-content-color` |
| `--mod-button-text-align` |
| `--mod-button-text-align-with-icon` |
| `--mod-button-top-to-icon` |
| `--mod-button-top-to-text` |
| `--mod-focus-indicator-gap` |
| `--mod-line-height-100` |
Expand Down
2 changes: 1 addition & 1 deletion components/button/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"main": "dist/index-vars.css",
"peerDependencies": {
"@spectrum-css/icon": ">=4",
"@spectrum-css/icon": ">=6",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🥳 Thanks for capturing this requirement update!

"@spectrum-css/progresscircle": ">=2",
"@spectrum-css/tokens": ">=13"
},
Expand Down
Loading