Skip to content

Commit

Permalink
[IndexFilters] consolidate se23 styles and logic (#10230)
Browse files Browse the repository at this point in the history
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #9938 

### WHAT is this pull request doing?

* Consolidate se23 styles 
* Consolidate se23 logic
* Remove custom `FilterButton` and replace it with direct invocations of
the Polaris Button instead. This seems safe to do because:
* even though we export `FilterButton` from the root of `IndexFilters`,
we do not actually expose it directly from the `polaris-react`
[entrypoint](https://github.com/Shopify/polaris/blob/2c718dc3ecf9393119cd1e08c387b648db78449a/polaris-react/src/index.ts#L190).
* AFAIK, noone is reaching into our cjs/esm builds to grab`FilterButton`
(see
[grokt](https://grokt.shopify.io/results?q=%40shopify%2Fpolaris%2F.*%2Fcomponents%2FIndexFilters))
* Just to be sure, also doesn't look like anyone is importing
`FilterButton` from @shopify/polaris either. (see
[grokt](https://grokt.shopify.io/results?q=FilterButton))
* That said I can see an argument being made for providing convenience
wrappers to alleviate repetitive config, so deferring to the team. 👍
Happy to revert if necessary 👍

### 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
  • Loading branch information
gwyneplaine committed Aug 28, 2023
1 parent e18ca90 commit a573e55
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 203 deletions.
5 changes: 5 additions & 0 deletions .changeset/wild-dodos-refuse.md
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

`IndexFilter` remove custom `FilterButton` in favor of directly invoking the Polaris `Button` component.
22 changes: 5 additions & 17 deletions polaris-react/src/components/IndexFilters/IndexFilters.scss
Expand Up @@ -30,22 +30,17 @@ $spinner-size: 20px;

.TabsWrapper {
flex: 1;
height: var(--p-space-12);
height: 44px;

#{$se23} & {
height: 44px;

@media #{$p-breakpoints-md-down} {
height: var(--p-space-12);
}
@media #{$p-breakpoints-md-down} {
height: var(--p-space-12);
}
}

.SmallScreenTabsWrapper {
overflow: hidden;
padding: var(--p-space-1) var(--p-space-0) var(--p-space-2) var(--p-space-3);
padding: 0;
height: auto;

&.TabsWrapperLoading {
position: relative;
Expand Down Expand Up @@ -90,11 +85,12 @@ $spinner-size: 20px;
gap: var(--p-space-2);
align-items: center;
justify-content: flex-start;
padding: var(--p-space-2) var(--p-space-3) var(--p-space-2) 0;
padding: var(--p-space-1_5-experimental) var(--p-space-2);

@media #{$p-breakpoints-md-down} {
position: relative;
padding: var(--p-space-3) var(--p-space-4);
height: 3rem;
border-left: var(--p-border-width-1) solid var(--p-color-border-subdued);
gap: var(--p-space-2);

Expand All @@ -115,14 +111,6 @@ $spinner-size: 20px;
// stylelint-enable
}
}

#{$se23} & {
padding: var(--p-space-1_5-experimental) var(--p-space-2);

@media #{$p-breakpoints-md-down} {
height: 3rem;
}
}
}

.ActionWrap svg {
Expand Down
8 changes: 1 addition & 7 deletions polaris-react/src/components/IndexFilters/IndexFilters.tsx
Expand Up @@ -13,7 +13,6 @@ import type {FiltersProps} from '../Filters';
import {Tabs} from '../Tabs';
import type {TabsProps} from '../Tabs';
import {useBreakpoints} from '../../utilities/breakpoints';
import {useFeatures} from '../../utilities/features';

import {useIsSticky} from './hooks';
import {
Expand Down Expand Up @@ -145,7 +144,6 @@ export function IndexFilters({
setFalse: setFiltersUnFocused,
setTrue: setFiltersFocused,
} = useToggle(false);
const {polarisSummerEditions2023} = useFeatures();

useOnValueChange(mode, (newMode) => {
if (newMode === IndexFiltersMode.Filtering) {
Expand Down Expand Up @@ -435,11 +433,7 @@ export function IndexFilters({
borderlessQueryField
closeOnChildOverlayClick={closeOnChildOverlayClick}
>
<InlineStack
gap={polarisSummerEditions2023 ? '2' : '3'}
align="start"
blockAlign="center"
>
<InlineStack gap="2" align="start" blockAlign="center">
<div
style={{
...defaultStyle,
Expand Down
@@ -1,14 +1,10 @@
@import '../../../../styles/common';

.Container {
border-bottom: var(--p-border-width-1) solid var(--p-color-border-subdued);
border-bottom: var(--p-border-width-1) solid var(--p-color-border);
border-top-left-radius: var(--p-border-radius-2);
border-top-right-radius: var(--p-border-radius-2);
background: var(--p-color-bg);

#{$se23} & {
border-bottom: var(--p-border-width-1) solid var(--p-color-border);
}
}

@media #{$p-breakpoints-sm-down} {
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Expand Up @@ -6,8 +6,7 @@ import {Icon} from '../../../Icon';
import {Tooltip} from '../../../Tooltip';
import {Text} from '../../../Text';
import {InlineStack} from '../../../InlineStack';
import {FilterButton} from '../FilterButton';
import {useFeatures} from '../../../../utilities/features';
import {Button} from '../../../Button';

export interface SearchFilterButtonProps {
onClick: () => void;
Expand All @@ -28,27 +27,22 @@ export function SearchFilterButton({
hideFilters,
hideQueryField,
}: SearchFilterButtonProps) {
const {polarisSummerEditions2023: se23} = useFeatures();

const iconMarkup = (
<InlineStack gap="0">
{hideQueryField ? null : <Icon source={SearchMinor} tone="base" />}
{hideFilters ? null : <Icon source={FilterMinor} tone="base" />}
</InlineStack>
);

const childMarkup = !se23 ? iconMarkup : null;

const activator = (
<div style={style}>
<FilterButton
<Button
size="slim"
onClick={onClick}
label={label}
disabled={disabled}
icon={se23 ? iconMarkup : undefined}
>
{childMarkup}
</FilterButton>
icon={iconMarkup}
accessibilityLabel={label}
/>
</div>
);

Expand Down
Expand Up @@ -2,15 +2,13 @@ import React, {useState, useMemo} from 'react';
import {SortMinor} from '@shopify/polaris-icons';

import {useI18n} from '../../../../utilities/i18n';
import {Icon} from '../../../Icon';
import {Popover} from '../../../Popover';
import {ChoiceList} from '../../../ChoiceList';
import type {ChoiceListProps} from '../../../ChoiceList';
import {Tooltip} from '../../../Tooltip';
import {Box} from '../../../Box';
import type {SortButtonChoice} from '../../types';
import {FilterButton} from '../FilterButton';
import {useFeatures} from '../../../../utilities/features';
import {Button} from '../../../Button';

import {DirectionButton} from './components';

Expand All @@ -37,7 +35,6 @@ export function SortButton({
onChangeDirection,
}: SortButtonProps) {
const i18n = useI18n();
const {polarisSummerEditions2023} = useFeatures();

const [active, setActive] = useState(false);
const [selectedValueKey, selectedDirection] = selected[0].split(' ');
Expand Down Expand Up @@ -97,25 +94,21 @@ export function SortButton({
return currentKey === selectedValueKey;
});

const iconMarkup = polarisSummerEditions2023 ? SortMinor : undefined;
const childMarkup = !polarisSummerEditions2023 ? (
<Icon source={SortMinor} tone="base" />
) : null;

const sortButton = (
<Tooltip
content={i18n.translate('Polaris.IndexFilters.SortButton.tooltip')}
preferredPosition="above"
hoverDelay={400}
>
<FilterButton
icon={iconMarkup}
<Button
size="slim"
icon={SortMinor}
onClick={handleClick}
label={i18n.translate('Polaris.IndexFilters.SortButton.ariaLabel')}
disabled={disabled}
>
{childMarkup}
</FilterButton>
accessibilityLabel={i18n.translate(
'Polaris.IndexFilters.SortButton.ariaLabel',
)}
/>
</Tooltip>
);

Expand All @@ -130,11 +123,10 @@ export function SortButton({
>
<Box
minWidth="148px"
padding={polarisSummerEditions2023 ? undefined : '4'}
paddingInlineStart={polarisSummerEditions2023 ? '3' : undefined}
paddingInlineEnd={polarisSummerEditions2023 ? '3' : undefined}
paddingBlockStart={polarisSummerEditions2023 ? '2' : undefined}
paddingBlockEnd={polarisSummerEditions2023 ? '2' : undefined}
paddingInlineStart="3"
paddingInlineEnd="3"
paddingBlockStart="2"
paddingBlockEnd="2"
borderBlockEndWidth="1"
borderColor="border-subdued"
>
Expand All @@ -146,15 +138,10 @@ export function SortButton({
/>
</Box>
<Box
padding={polarisSummerEditions2023 ? undefined : '4'}
paddingInlineStart={
polarisSummerEditions2023 ? '1_5-experimental' : undefined
}
paddingInlineEnd={
polarisSummerEditions2023 ? '1_5-experimental' : undefined
}
paddingBlockStart={polarisSummerEditions2023 ? '2' : undefined}
paddingBlockEnd={polarisSummerEditions2023 ? '2' : undefined}
paddingInlineStart="1_5-experimental"
paddingInlineEnd="1_5-experimental"
paddingBlockStart="2"
paddingBlockEnd="2"
>
<DirectionButton
direction="asc"
Expand Down
Expand Up @@ -2,9 +2,9 @@

.DirectionButton {
// stylelint-disable-next-line -- This mixin cannot be replaced with a single token
@include focus-ring;
border-radius: var(--p-border-radius-1);
padding: var(--p-space-05);
@include focus-ring($size: 'wide');
border-radius: var(--p-border-radius-2);
padding: var(--p-space-1) var(--p-space-3) var(--p-space-1) var(--p-space-2);
font-size: var(--p-font-size-100);
display: flex;
align-items: center;
Expand All @@ -15,14 +15,7 @@
background: none;
text-align: left;

#{$se23} & {
// stylelint-disable-next-line -- se23 overrides
@include focus-ring($size: 'wide');
border-radius: var(--p-border-radius-2);
padding: var(--p-space-1) var(--p-space-3) var(--p-space-1) var(--p-space-2);
}

#{$se23} &:hover {
&:hover {
background-color: var(--p-color-bg-transparent-hover-experimental);
}

Expand All @@ -34,34 +27,20 @@
outline: 0;

// stylelint-disable-next-line -- This mixin cannot be replaced with a single token
@include focus-ring($style: 'focused');

#{$se23} & {
// stylelint-disable-next-line -- se23 overrides
@include focus-ring($size: 'wide', $style: 'focused');
}
@include focus-ring($size: 'wide', $style: 'focused');
}
}

.DirectionButton-active {
background: var(--p-color-bg-interactive-selected);
color: var(--p-color-text-interactive);

#{$se23} & {
color: var(--p-color-text);
background: var(--p-color-bg-transparent-active-experimental);
}
color: var(--p-color-text);
background: var(--p-color-bg-transparent-active-experimental);
}

.Label {
flex: 1;
margin-left: var(--p-space-1);

#{$se23} & {
color: var(--p-color-text);
margin-left: var(--p-space-05);
font-size: var(--p-font-size-75);
font-weight: var(--p-font-weight-medium);
line-height: var(--p-font-line-height-1);
}
color: var(--p-color-text);
margin-left: var(--p-space-05);
font-size: var(--p-font-size-75);
font-weight: var(--p-font-weight-medium);
line-height: var(--p-font-line-height-1);
}

0 comments on commit a573e55

Please sign in to comment.