[ButtonGroup] Add optional nowrap property to ButtonGroup #8033

allisonjanes-shopify · 2023-01-11T21:26:55Z

This PR is non-breaking and adds an optional noWrap property to the ButtonGroup component which prevents the buttons in a ButtonGroup from wrapping onto a new line. This change will help with alignment issues we have been having on the new Metaobjects Index page encountered with very long Metaobject Type names.

A noWrap option will allow for the title object to wrap sooner instead of forcing the action buttons onto 2 lines.

WHY are these changes introduced?

To address alignment issues on the new Metaobjects page. Since we want our buttons in line with our Metaobjects Type label (which might be long), we would prefer to set the ButtonGroup to noWrap and force the label to wrap.

WHAT is this pull request doing?

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide

github-actions · 2023-01-11T21:30:59Z

size-limit report 📦

Path	Size
polaris-react-cjs	212.25 KB (+0.01% 🔺)
polaris-react-esm	137.21 KB (+0.02% 🔺)
polaris-react-esnext	192.78 KB (+0.03% 🔺)
polaris-react-css	41.99 KB (+0.02% 🔺)

chloerice · 2023-01-11T21:58:00Z

polaris-react/src/components/ButtonGroup/ButtonGroup.tsx

  segmented,
  fullWidth,
  connectedTop,
+  noWrap,


Suggested change

noWrap,

noWrap,

}: ButtonGroupProps) {

const className = classNames(

styles.ButtonGroup,

spacing && styles[spacing],

segmented && styles.segmented,

fullWidth && styles.fullWidth,

noWrap && style.noWrap,

);

I see now how this is wrapped around the {contents}. My div was redundant. Updated.

chloerice · 2023-01-11T21:58:50Z

polaris-react/src/components/ButtonGroup/ButtonGroup.tsx

+      data-buttongroup-no-wrap={noWrap}
    >
-      {contents}
+      <div className={classNames(noWrap && styles.noWrap)}>{contents}</div>


Suggested change

<div className={classNames(noWrap && styles.noWrap)}>{contents}</div>

{contents}

chloerice · 2023-01-11T22:09:21Z

polaris-react/src/components/ButtonGroup/ButtonGroup.scss

 }
+
+.noWrap {
+  display: flex;
+  flex-wrap: nowrap;
+}


I think the bug highlighted by the visual regression snapshots can be fixed by moving this class in with the rest of the subclasses 👀

Suggested change

}

.noWrap {

display: flex;

flex-wrap: nowrap;

}

.noWrap {

display: flex;

flex-wrap: nowrap;

}

}

So, moving this up into the above brackets would move it inside the .loose class, no?

If I look at the .scss file, none of these sub classes actually seem to be inside .ButtonGroup

Ah yeah sorry! Made the suggestion on scanning the file here, but no the subclasses aren't nested so there's something else going on 👀 It's odd because the .segmented class also makes the button group not wrap so I'd expect what you have to be enough, but I haven't had time to check out the branch and poke around.

I played around with this in the Playground and it seems to be working as expected now that I've moved the class into the proper div on the tsx page. I also updated the ButtonGroup NoWrap story to show the difference between default and nowrap. Let me know if that can be improved. Thanks @chloerice!

chloerice

This looks good to go 🎩 👍🏽 Thanks for contributing @allisonjanes-shopify! 🎉

chloerice · 2023-01-13T17:37:22Z

.changeset/happy-dragons-repair.md

+'@shopify/polaris': minor
+---
+
+Add optional noWrap property to ButtonGroup


Suggested change

Add optional noWrap property to ButtonGroup

Added an optional `noWrap` prop to `ButtonGroup`

@allisonjanes-shopify

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris@10.21.0 ### Minor Changes - [#8033](#8033) [`a2eca1d4d`](a2eca1d) Thanks [@allisonjanes-shopify](https://github.com/allisonjanes-shopify)! - Added an optional `noWrap` prop to `ButtonGroup` - [#8016](#8016) [`ca77f0bc8`](ca77f0b) Thanks [@itwasmattgregg](https://github.com/itwasmattgregg)! - Added the ability for breadcrumbs to be passed as a singular object instead of an array to allow support for upcoming v11 changes ### Patch Changes - [#8059](#8059) [`4b470fc98`](4b470fc) Thanks [@LauraAubin](https://github.com/LauraAubin)! - Fix vertical alignment for sortable headers with custom content on the IndexTable ## @shopify/plugin-polaris@0.0.28 ## polaris.shopify.com@0.28.5 ### Patch Changes - Updated dependencies \[[`4b470fc98`](4b470fc), [`a2eca1d4d`](a2eca1d), [`ca77f0bc8`](ca77f0b)]: - @shopify/polaris@10.21.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

This PR is non-breaking and adds an optional noWrap property to the ButtonGroup component which prevents the buttons in a ButtonGroup from wrapping onto a new line. This change will help with alignment issues we have been having on the new Metaobjects Index page encountered with very long Metaobject Type names. <img width="556" alt="Screenshot 2023-01-11 at 4 13 12 PM" src="https://user-images.githubusercontent.com/109357057/211918907-9632b84b-1f32-4b5b-80d4-75b2130f0022.png"> A `noWrap` option will allow for the title object to wrap sooner instead of forcing the action buttons onto 2 lines. ### WHY are these changes introduced? To address alignment issues on the new Metaobjects page. Since we want our buttons in line with our Metaobjects `Type` label (which might be long), we would prefer to set the `ButtonGroup` to `noWrap` and force the label to wrap.  ### WHAT is this pull request doing?  ### 🎩 checklist - [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) - [x] 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

@allisonjanes-shopify

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris@10.21.0 ### Minor Changes - [Shopify#8033](Shopify#8033) [`a2eca1d4d`](Shopify@a2eca1d) Thanks [@allisonjanes-shopify](https://github.com/allisonjanes-shopify)! - Added an optional `noWrap` prop to `ButtonGroup` - [Shopify#8016](Shopify#8016) [`ca77f0bc8`](Shopify@ca77f0b) Thanks [@itwasmattgregg](https://github.com/itwasmattgregg)! - Added the ability for breadcrumbs to be passed as a singular object instead of an array to allow support for upcoming v11 changes ### Patch Changes - [Shopify#8059](Shopify#8059) [`4b470fc98`](Shopify@4b470fc) Thanks [@LauraAubin](https://github.com/LauraAubin)! - Fix vertical alignment for sortable headers with custom content on the IndexTable ## @shopify/plugin-polaris@0.0.28 ## polaris.shopify.com@0.28.5 ### Patch Changes - Updated dependencies \[[`4b470fc98`](Shopify@4b470fc), [`a2eca1d4d`](Shopify@a2eca1d), [`ca77f0bc8`](Shopify@ca77f0b)]: - @shopify/polaris@10.21.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

allisonjanes-shopify requested a review from MstrZhang January 11, 2023 21:26

allisonjanes-shopify self-assigned this Jan 11, 2023

chloerice reviewed Jan 11, 2023

View reviewed changes

allisonjanes-shopify force-pushed the aj/add-nowrap-to-button-group branch from 8f0b9f1 to 2b7a892 Compare January 12, 2023 15:16

allisonjanes-shopify requested a review from chloerice January 12, 2023 15:52

chloerice approved these changes Jan 13, 2023

View reviewed changes

Add optional nowrap property to ButtonGroup

d8d2bf0

allisonjanes-shopify force-pushed the aj/add-nowrap-to-button-group branch from 2b7a892 to d8d2bf0 Compare January 13, 2023 18:47

MstrZhang approved these changes Jan 16, 2023

View reviewed changes

allisonjanes-shopify merged commit a2eca1d into main Jan 16, 2023

allisonjanes-shopify deleted the aj/add-nowrap-to-button-group branch January 16, 2023 21:05

shopify-github-actions-access bot mentioned this pull request Jan 16, 2023

Version Packages #8060

Merged

gwyneplaine mentioned this pull request Feb 14, 2023

8248/fix toc layout #8338

Merged

-  noWrap,
+  noWrap,
+}: ButtonGroupProps) {
+  const className = classNames(
+    styles.ButtonGroup,
+    spacing && styles[spacing],
+    segmented && styles.segmented,
+    fullWidth && styles.fullWidth,
+    noWrap && style.noWrap,
+  );

	<div className={classNames(noWrap && styles.noWrap)}>{contents}</div>
	{contents}

	Add optional noWrap property to ButtonGroup
	Added an optional `noWrap` prop to `ButtonGroup`

[ButtonGroup] Add optional nowrap property to ButtonGroup #8033

[ButtonGroup] Add optional nowrap property to ButtonGroup #8033

Uh oh!

Conversation

allisonjanes-shopify commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

🎩 checklist

Uh oh!

github-actions bot commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chloerice Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chloerice left a comment

Choose a reason for hiding this comment

Uh oh!

chloerice Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

allisonjanes-shopify commented Jan 11, 2023 •

edited

Loading

github-actions bot commented Jan 11, 2023 •

edited

Loading

chloerice Jan 11, 2023 •

edited

Loading

chloerice Jan 13, 2023 •

edited

Loading