Skip to content
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

[Filters] Remove unused disabled states from FilterPill #10227

Closed
gwyneplaine opened this issue Aug 25, 2023 · 0 comments · Fixed by #10228
Closed

[Filters] Remove unused disabled states from FilterPill #10227

gwyneplaine opened this issue Aug 25, 2023 · 0 comments · Fixed by #10228
Assignees
@gwyneplaine
Labels
Bug Something is broken and not working as intended in the system.

Comments

@gwyneplaine
Copy link
Contributor

As of #8876 the Filters component no longer renders FilterPill's with the disabled state enabled.
Unfortunately there's still remnant logic for setting a disabled state on the underlying UnstyledButton implementation, as well as disabled styles in FilterPill.scss.

As this is now dead code it should be removed.
@gwyneplaine gwyneplaine self-assigned this Aug 25, 2023
@gwyneplaine gwyneplaine added Bug Something is broken and not working as intended in the system. #gsd:36579 and removed Feature request untriaged labels Aug 25, 2023
@gwyneplaine gwyneplaine mentioned this issue Aug 25, 2023
Merged
5 tasks
@gwyneplaine gwyneplaine linked a pull request Aug 25, 2023 that will close this issue
Filters remove unused disabled states and styles from FilterPill #10228
Merged
5 tasks
gwyneplaine added a commit that referenced this issue Aug 28, 2023
@gwyneplaine
Filters remove unused disabled states and styles from FilterPill (#…
e18ca90 
…10228)

Fixes #10227 

### WHAT is this pull request doing?

* Removes unused disabled states in `FilterPill.scss`
* Removes unused disabled states in `FilterPill.tsx`

<!-- ℹ️ 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
sophschneider pushed a commit that referenced this issue Sep 19, 2023
@gwyneplaine @sophschneider
Filters remove unused disabled states and styles from FilterPill (#…
b0d1e7e 
…10228)

Fixes #10227 

### WHAT is this pull request doing?

* Removes unused disabled states in `FilterPill.scss`
* Removes unused disabled states in `FilterPill.tsx`

<!-- ℹ️ 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
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this issue Apr 22, 2024
@gwyneplaine
Filters remove unused disabled states and styles from FilterPill (S…
c1e36bc 
…hopify#10228)

Fixes Shopify#10227 

### WHAT is this pull request doing?

* Removes unused disabled states in `FilterPill.scss`
* Removes unused disabled states in `FilterPill.tsx`

<!-- ℹ️ 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gwyneplaine gwyneplaine

Labels
Bug Something is broken and not working as intended in the system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Filters remove unused disabled states and styles from FilterPill Shopify/polaris
1 participant
@gwyneplaine