Skip to content

Conversation

@itwasmattgregg
Copy link
Contributor

WHY are these changes introduced?

For an upcoming change to breadcrumbs where arrays will no longer be allowed, this change allows current consumers to begin making that transition.

For a while now only the last item in the array has been shown in the breadcrumb component. Changing the interface away from accepting arrays will make the props more indicative of the final component output and easier to understand at a glance.

WHAT is this pull request doing?

It just changes the interface to allow arrays and non-arrays. Tests have been added for this as well.

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Jan 10, 2023

size-limit report 📦

Path Size
polaris-react-cjs 212.22 KB (+0.01% 🔺)
polaris-react-esm 137.2 KB (+0.03% 🔺)
polaris-react-esnext 192.73 KB (+0.02% 🔺)
polaris-react-css 41.98 KB (0%)

@itwasmattgregg
Copy link
Contributor Author

It does feel really weird checking if breadcrumbs is an array and array has length OR if breadcrumbs. But I can't think of a simpler check. It's temporary for now

Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

Just one comment around empty arrays. With how often this component is used in the admin we want to be as explicit with our checks as possible

Co-Authored-By: Kyle Durand <kyledurand@users.noreply.github.com>
@itwasmattgregg itwasmattgregg force-pushed the breadcrumb-single-optional branch from cffcf6c to b56903a Compare January 12, 2023 23:07
Copy link
Contributor

@chazdean chazdean left a comment

Choose a reason for hiding this comment

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

LGTM, my only question would be if it would be necessary to check for empty objects as well?

Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

👍

@itwasmattgregg itwasmattgregg merged commit ca77f0b into main Jan 16, 2023
@itwasmattgregg itwasmattgregg deleted the breadcrumb-single-optional branch January 16, 2023 21:07
ginabak pushed a commit that referenced this pull request Jan 16, 2023
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>
itwasmattgregg added a commit that referenced this pull request Jan 17, 2023
@gwyneplaine gwyneplaine mentioned this pull request Feb 14, 2023
itwasmattgregg added a commit that referenced this pull request Feb 23, 2023
…Breadcrumbs (#8135)

### WHY are these changes introduced?

As a followup to #8016 in
preparation for v11 where breadcrumb becomes singular
(#7990).

I should have originally done this and deprecated breadcrumbs but at the
time we had a different upgrade path in mind. After collaborating with a
number of folks we determined deprecating breadcrumbs officially and
moving to a singular breadcrumb in a `backAction` prop for v11 was the
best path.

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [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)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] 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 <chloerice@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
### WHY are these changes introduced?

For an upcoming change to breadcrumbs where arrays will no longer be
allowed, this change allows current consumers to begin making that
transition.

For a while now only the last item in the array has been shown in the
breadcrumb component. Changing the interface away from accepting arrays
will make the props more indicative of the final component output and
easier to understand at a glance.

### WHAT is this pull request doing?

It just changes the interface to allow arrays and non-arrays. Tests have
been added for this as well.

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] 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: Kyle Durand <kyledurand@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
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>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
…Breadcrumbs (Shopify#8135)

### WHY are these changes introduced?

As a followup to Shopify#8016 in
preparation for v11 where breadcrumb becomes singular
(Shopify#7990).

I should have originally done this and deprecated breadcrumbs but at the
time we had a different upgrade path in mind. After collaborating with a
number of folks we determined deprecating breadcrumbs officially and
moving to a singular breadcrumb in a `backAction` prop for v11 was the
best path.

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [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)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] 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 <chloerice@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants