Skip to content

Conversation

tmlayton
Copy link
Contributor

@tmlayton tmlayton commented Jun 20, 2019

WHY are these changes introduced?

Closes #1167
Closes #1510

WHAT is this pull request doing?

Moving the <Filters /> component to Polaris

Documentation

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Page} from '@shopify/polaris';

interface State {}

export default class Playground extends React.Component<never, State> {
  render() {
    return (
      <Page title="Playground">
        {/* Add the code you want to test here */}
      </Page>
    );
  }
}

🎩 checklist

Known issues

@danrosenthal did some digging and looks like any children which cause a reflow (here is all of the things that can cause a reflow) during the entering animation will force the animation to skip to the end. For more details see reactjs/react-transition-group#382. For now this is a won’t fix.

Related todos

The following will be done in other PRs:

  • Move ResourceList.Item to its own component named ResourceItem
  • Update ResourceList examples to use ResourceFilters
  • Deprecate FilterControl

@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 20, 2019 00:49 Inactive
@ry5n
Copy link
Contributor

ry5n commented Jun 20, 2019

😍

@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 21, 2019 03:59 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 21, 2019 16:44 Inactive
Copy link

@danrosenthal danrosenthal left a comment

Choose a reason for hiding this comment

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

😍

@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 21, 2019 16:50 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 21, 2019 17:02 Inactive
@tmlayton tmlayton changed the title Resource filters Filters component Jun 21, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 21, 2019 23:23 Inactive
@tmlayton
Copy link
Contributor Author

I went ahead an updated the component to be called Filters and provided an example of using it with data table and resource list.

@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 24, 2019 12:14 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 25, 2019 18:40 Inactive
Copy link
Contributor

@sadiesaurus sadiesaurus left a comment

Choose a reason for hiding this comment

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

Nice work! 💖 A few comments and questions.

I didn't see the related components section. Did we decide it was best to not include it here?

@ghost ghost added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Jun 26, 2019
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 07:49 Inactive
@ghost ghost removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Jun 26, 2019
@tmlayton
Copy link
Contributor Author

I didn't see the related components section. Did we decide it was best to not include it here?

Yes, the format for related components is to suggest other components if trying to do something related, but we have no other filter component so it didn’t feel right to include.

@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 18:58 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 18:58 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 18:59 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 18:59 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 19:04 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 19:08 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 20:10 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 20:25 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1718 June 26, 2019 20:40 Inactive
@tmlayton
Copy link
Contributor Author

tmlayton commented Jun 27, 2019

Okay going to merge! If there are any other changes to docs we can do them in follow-up PRs.

@tmlayton tmlayton merged commit 40207e5 into master Jun 27, 2019
@tmlayton tmlayton deleted the resource-filters branch June 27, 2019 02:50
@dleroux dleroux temporarily deployed to production July 9, 2019 15:19 Inactive
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.

[List filters] Move list filter component to Polaris [List filters] Use Sheet from Polaris

6 participants