Skip to content

[polaris.shopify.com][/foundations][/patterns] Locations > Resource picking #6744

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

Jump to bottom
Merged
merged 10 commits into from
Jul 27, 2022
Merged

[polaris.shopify.com][/foundations][/patterns] Locations > Resource picking #6744

merged 10 commits into from
Jul 27, 2022

Conversation

@chloerice
Copy link
Member

@chloerice chloerice commented Jul 22, 2022 •
edited
Loading

WHY are these changes introduced?

Follow up to #6596

WHAT is this pull request doing?

  • Updates the /foundations/patterns/locations Locations pattern doc to document Pickers with Locations and Customer segments as examples.
  • Adds storybook and style guide examples for searchable listbox to Listbox, Popover, and Sheet

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx: 
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>
  );
}

🎩 checklist

@chloerice chloerice changed the title [polaris.shopify.com][/foundations][/patterns] Scaffold IA for locations pattern update [polaris.shopify.com][/foundations][/patterns] Locations > Resource picking Jul 22, 2022
@chloerice chloerice force-pushed the picker-pattern-doc branch 12 times, most recently from 01b649f to ab31c29 Compare July 26, 2022 06:21
@ghost ghost added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Jul 26, 2022
@chloerice chloerice force-pushed the picker-pattern-doc branch from 8c041a9 to 0d9f921 Compare July 26, 2022 08:56
@ghost ghost removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Jul 26, 2022
@chloerice chloerice force-pushed the picker-pattern-doc branch 3 times, most recently from 52fa4e9 to 75205de Compare July 26, 2022 09:08
@chloerice chloerice marked this pull request as ready for review July 26, 2022 10:16
@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2022 •
edited
Loading

size-limit report 📦

Path Size
polaris-react-cjs 198.27 KB (+0.01% 🔺)
polaris-react-esm 133.14 KB (-0.01% 🔽)
polaris-react-esnext 188.28 KB (-0.01% 🔽)
polaris-react-css 41.75 KB (0%)

@chloerice chloerice requested a review from yurm04 July 26, 2022 14:51
@chloerice chloerice force-pushed the picker-pattern-doc branch from 72e1b75 to 65793d0 Compare July 26, 2022 20:18
@chloerice chloerice force-pushed the picker-pattern-doc branch 7 times, most recently from e3c791e to da6305f Compare July 26, 2022 21:56
chloerice
chloerice commented Jul 26, 2022
View reviewed changes
polaris-react/src/components/PortalsManager/components/PortalsContainer/PortalsContainer.tsx Outdated
ref: RefObject<HTMLDivElement>,
) {
return <div id="PolarisPortalsContainer" ref={ref} />;
const uniqueId = useUniqueId('PortalsContainer');
Copy link
Member Author

@chloerice chloerice Jul 26, 2022

Choose a reason for hiding this comment

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

(Fixes duplicate ID pa11y CI errors)

Copy link
Member

@alex-page alex-page Jul 26, 2022

Choose a reason for hiding this comment

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

This should be resolved with a merge from main. This happened when two AppProviders wrapped a component as a bug caused by me 😭

Copy link
Member Author

@chloerice chloerice Jul 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ahh okay thank you!! This CI check was driving me nuts 😁

Copy link
Member

@alex-page alex-page Jul 26, 2022

Choose a reason for hiding this comment

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

My bad Chloe.

Copy link
Member Author

@chloerice chloerice Jul 26, 2022

Choose a reason for hiding this comment

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

No worries!!!

Copy link
Member Author

@chloerice chloerice Jul 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm the saga continues lol, CI is still complaining about duplicate IDs for the PortalsContainer, even though only one example and only one PortalsContainer ever gets rendered at a time 🧐

Copy link
Member

@alex-page alex-page Jul 26, 2022

Choose a reason for hiding this comment

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

It looks good now. When there are zero changes in polaris-react the a11y test doesn't run.

Copy link
Member

@alex-page alex-page Jul 26, 2022

Choose a reason for hiding this comment

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

https://github.com/Shopify/polaris/blob/fdf4d836dccedd4578d094fa216c5460780064cc/.github/workflows/ci-a11y-vrt.yml#L3-L16

Copy link
Member

@alex-page alex-page Jul 27, 2022

Choose a reason for hiding this comment

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

Ok I am silly. Fixed here 31b8603

Copy link
Member

@alex-page alex-page Jul 27, 2022

Choose a reason for hiding this comment

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

Sorry for the inconvenience Chloe 😬

@chloerice chloerice force-pushed the picker-pattern-doc branch from 441c028 to e1aec19 Compare July 26, 2022 22:48
@chloerice chloerice merged commit 6365532 into main Jul 27, 2022
@chloerice chloerice deleted the picker-pattern-doc branch July 27, 2022 15:42
@github-actions github-actions bot mentioned this pull request Jul 27, 2022
Merged
@selenehinkley selenehinkley mentioned this pull request Aug 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alex-page alex-page Awaiting requested review from alex-page

@yurm04 yurm04 Awaiting requested review from yurm04

1 more reviewer

@martenbjork martenbjork martenbjork approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@chloerice chloerice

@selenehinkley selenehinkley

@tracytees tracytees

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chloerice @martenbjork @alex-page @selenehinkley @tracytees