Skip to content

Commit

Permalink
[Button] Fix styling for fullWidth buttons in ButtonGroup (#11499)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Resolves
[#1382](https://github.com/Shopify/polaris-internal/issues/1382).

### WHAT is this pull request doing?
Fixes width for Button inside a `ButtonGroup` where `fullWidth` prop is
applied.
  <details>
    <summary>ButtonGroup — before</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/f8bff26b-9246-44d0-81ae-39f6a69f5e02"
alt="ButtonGroup — before">
  </details>
  <details>
    <summary>ButtonGroup — after</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/3b58d993-dd1a-476f-9200-3650a1fcbd98"
alt="ButtonGroup — after">
  </details>

Fixes overlapping focus styles when Button rendered as a child of
Tooltip.
  <details>
    <summary>Tooltip focused — before</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/85a249e6-1a24-4a4c-8225-0ef3d7e9aba8"
alt="Tooltip focused — before">
  </details>
  <details>
    <summary>Tooltip focused — after</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/3b871c74-11e6-45b1-8c5d-7c9f4f7cfcf5"
alt="Tooltip focused — after">
  </details>

### How to 🎩


[Spin](https://admin.web.segmented-button-fix.lo-kim.us.spin.dev/store/shop1/settings/branding)

[Full width buttons —
prod](https://storybook.polaris.shopify.com/?path=/story/all-components-tooltip--visible-only-with-child-interaction)
[Full width buttons —
pr](https://5d559397bae39100201eedc1-fxudyneyuk.chromatic.com/?path=/story/all-components-tooltip--visible-only-with-child-interaction)

[Overlapping focus styles —
prod](https://storybook.polaris.shopify.com/?path=/story/all-components-tooltip--with-suffix)
[Overlapping focus styles —
pr](https://5d559397bae39100201eedc1-fxudyneyuk.chromatic.com/?path=/story/all-components-tooltip--with-suffix)

[ButtonGroup segmented -
pr](https://5d559397bae39100201eedc1-fxudyneyuk.chromatic.com/?path=/story/all-components-buttongroup--with-segmented-buttons)

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Chloe Rice <[email protected]>
Co-authored-by: Sam Rose <[email protected]>
  • Loading branch information
3 people authored Jan 26, 2024
1 parent 138e1aa commit a6ac992
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/dry-chicken-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Fixed layout and focus styling for `Button` inside `ButtonGroup` with `fullWidth`
13 changes: 13 additions & 0 deletions polaris-react/src/components/Button/Button.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@
color: var(--pc-button-color_hover);
outline: var(--p-border-width-050) solid var(--p-color-border-focus);
outline-offset: var(--p-space-025);

// Disable focus-ring mixin to prevent overlap of focus-ring and outline
&::after {
content: none;
}
}

.Button:disabled,
Expand Down Expand Up @@ -385,4 +390,12 @@
border-top-right-radius: var(--p-border-radius-0);
border-bottom-right-radius: var(--p-border-radius-0);
}

[data-buttongroup-full-width='true'] .Button {
width: 100%;

@media (--p-breakpoints-md-up) {
white-space: nowrap;
}
}
/* stylelint-enable -- selector-max-combinators */
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
line-height: 1;

&:not(:first-child) {
margin-left: calc(-0.5 * var(--p-space-025));
margin-left: calc(-1 * var(--p-space-025));
}
}

Expand Down
21 changes: 21 additions & 0 deletions polaris-react/src/components/ButtonGroup/ButtonGroup.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,27 @@ export function WithSegmentedButtons() {
Neptune
</Button>
</ButtonGroup>
<br />
<ButtonGroup variant="segmented" fullWidth>
<Button
pressed={activeButtonIndex === 8}
onClick={() => handleButtonClick(8)}
>
Bold
</Button>
<Button
pressed={activeButtonIndex === 9}
onClick={() => handleButtonClick(9)}
>
Italic
</Button>
<Button
pressed={activeButtonIndex === 10}
onClick={() => handleButtonClick(10)}
>
Underline
</Button>
</ButtonGroup>
</div>
);
}
Expand Down
1 change: 0 additions & 1 deletion polaris-react/src/components/Tooltip/Tooltip.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ export function WithSuffix() {
</Text>
</InlineStack>
}
activatorWrapper="div"
>
<Button>B</Button>
</Tooltip>
Expand Down

0 comments on commit a6ac992

Please sign in to comment.