From 99a57fd7af45209a75156570199f5e04eb42019c Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 18 Mar 2020 11:59:54 -0400 Subject: [PATCH 01/25] adds a hideMoreFiltersButton prop to the Filters component --- src/components/Filters/Filters.tsx | 4 ++++ .../ConnectedFilterControl.tsx | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 62bef8411c2..3be58c917c6 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -87,6 +87,8 @@ export interface FiltersProps { helpText?: string | React.ReactNode; /** Hide tags for applied filters */ hideTags?: boolean; + /** Hide "More filters" button **/ + hideMoreFiltersButton?: boolean; } type ComposedProps = FiltersProps & @@ -135,6 +137,7 @@ class FiltersInner extends React.Component { helpText, hideTags, newDesignLanguage, + hideMoreFiltersButton, } = this.props; const {resourceName} = this.context; const {open, readyForFocus} = this.state; @@ -249,6 +252,7 @@ class FiltersInner extends React.Component { rightAction={rightActionMarkup} auxiliary={children} disabled={disabled} + hideMoreFiltersButton={hideMoreFiltersButton} > 0 && + hideMoreFiltersButton ? null : ( + {rightAction} + ); + const rightMarkup = rightPopoverableActions ? (
{this.popoverFrom(actionsToRender)} @@ -105,7 +116,7 @@ export class ConnectedFilterControl extends React.Component< ref={this.moreFiltersButtonContainer} className={moreFiltersButtonContainerClassname} > - {rightAction} + {moreFilterButton}
) : null; From c9b59f13cbc1496681e12e0c33db39e6a26b2a5b Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 18 Mar 2020 17:09:53 -0400 Subject: [PATCH 02/25] removes the hideMoreFiltersButton prop --- src/components/Filters/Filters.tsx | 15 +++++------- .../ConnectedFilterControl.tsx | 23 ++++++++++--------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 3be58c917c6..99379d59faf 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -87,8 +87,6 @@ export interface FiltersProps { helpText?: string | React.ReactNode; /** Hide tags for applied filters */ hideTags?: boolean; - /** Hide "More filters" button **/ - hideMoreFiltersButton?: boolean; } type ComposedProps = FiltersProps & @@ -137,7 +135,6 @@ class FiltersInner extends React.Component { helpText, hideTags, newDesignLanguage, - hideMoreFiltersButton, } = this.props; const {resourceName} = this.context; const {open, readyForFocus} = this.state; @@ -246,13 +243,15 @@ class FiltersInner extends React.Component { plural: intl.translate('Polaris.ResourceList.defaultItemPlural'), }; + const transformedFilters = this.transformFilters(filters); + const filtersControlMarkup = ( transformedFilters.length} > { } } - private transformFilters( - filters: FilterInterface[], - ): ConnectedFilterControlProps['rightPopoverableActions'] | null { - const transformedActions: ConnectedFilterControlProps['rightPopoverableActions'] = []; + private transformFilters(filters: FilterInterface[]) { + const transformedActions: Required = []; getShortcutFilters(filters).forEach((filter) => { const {key, label, disabled} = filter; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index bf70e04d116..325be3349da 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -25,7 +25,7 @@ export interface ConnectedFilterControlProps { rightAction?: React.ReactNode; auxiliary?: React.ReactNode; disabled?: boolean; - hideMoreFiltersButton?: boolean; + forceShowMorefiltersButton?: boolean; } interface ComputedProperty { @@ -75,7 +75,7 @@ export class ConnectedFilterControl extends React.Component< rightPopoverableActions, rightAction, auxiliary, - hideMoreFiltersButton, + forceShowMorefiltersButton = true, } = this.props; const actionsToRender = @@ -89,16 +89,17 @@ export class ConnectedFilterControl extends React.Component< newDesignLanguage && styles.newDesignLanguage, ); - // If there's only one action, and it fits we don't show the "More filters" button - const moreFilterButton = + const actionsToRender = rightPopoverableActions && - rightPopoverableActions.length === 1 && - this.getActionsToRender(rightPopoverableActions).length > 0 && - hideMoreFiltersButton ? null : ( - {rightAction} - ); + this.getActionsToRender(rightPopoverableActions); + + const shouldRenderMoreFiltersButton = + forceShowMorefiltersButton || + (rightPopoverableActions && + actionsToRender && + rightPopoverableActions.length !== actionsToRender.length); - const rightMarkup = rightPopoverableActions ? ( + const rightMarkup = actionsToRender ? (
{this.popoverFrom(actionsToRender)}
@@ -116,7 +117,7 @@ export class ConnectedFilterControl extends React.Component< ref={this.moreFiltersButtonContainer} className={moreFiltersButtonContainerClassname} > - {moreFilterButton} + {shouldRenderMoreFiltersButton && {rightAction}} ) : null; From f47a8e40a4fe1c37e8c35523b3ccf0af944a9a34 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Fri, 27 Mar 2020 11:42:42 -0400 Subject: [PATCH 03/25] address Tim Layton comments --- .../ConnectedFilterControl.scss | 7 +++++++ .../ConnectedFilterControl/ConnectedFilterControl.tsx | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index 2bf617701c8..8675bb2eee7 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -83,6 +83,13 @@ $stacking-order: ( } } +.RightContainerWithoutMoreFilters { + .Item > * > * { + border-top-right-radius: border-radius() !important; + border-bottom-right-radius: border-radius() !important; + } +} + .MoreFiltersButtonContainer * { border-top-left-radius: 0 !important; border-bottom-left-radius: 0 !important; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index 325be3349da..551c35ab22d 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -99,8 +99,13 @@ export class ConnectedFilterControl extends React.Component< actionsToRender && rightPopoverableActions.length !== actionsToRender.length); + const RightContainerClassName = classNames( + styles.RightContainer, + !shouldRenderMoreFiltersButton && styles.RightContainerWithoutMoreFilters, + ); + const rightMarkup = actionsToRender ? ( -
+
{this.popoverFrom(actionsToRender)}
) : null; @@ -209,6 +214,10 @@ export class ConnectedFilterControl extends React.Component< if (actionWidth <= remainingWidth) { actionsToReturn.push(action); remainingWidth -= actionWidth; + } else { + // When we can't fit an action, we break the loop. + // The ones that didn't fit will be accessible through the "More filters" button + break; } } return actionsToReturn; From 6da3f158454e75229ec20e69523f219952f5544a Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Tue, 31 Mar 2020 10:29:02 -0400 Subject: [PATCH 04/25] address border-radius problem --- .../ConnectedFilterControl.scss | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index 8675bb2eee7..cac187df623 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -76,6 +76,19 @@ $stacking-order: ( } } +.RightContainerWithoutMoreFilters { + .Item:last-child > * > * { + border-top-right-radius: var( + --p-border-radius-base, + border-radius() + ) !important; + border-bottom-right-radius: var( + --p-border-radius-base, + border-radius() + ) !important; + } +} + .newDesignLanguage .RightContainer { .Item:first-of-type > * > * { border-top-left-radius: var(--p-border-radius-base) !important; From 65e63462922776e840588944825cf454cd07f493 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 11:50:04 -0400 Subject: [PATCH 05/25] adds tests and formats --- src/components/Filters/Filters.tsx | 4 +- .../tests/ConnectedFilterControl.test.tsx | 21 +++++++++- src/components/Filters/tests/Filters.test.tsx | 40 +++++++++++++++++++ src/components/Select/Select.scss | 10 ++--- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 99379d59faf..3f08fb817b8 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -517,7 +517,9 @@ class FiltersInner extends React.Component { } private transformFilters(filters: FilterInterface[]) { - const transformedActions: Required = []; + const transformedActions: Required< + ConnectedFilterControlProps['rightPopoverableActions'] + > = []; getShortcutFilters(filters).forEach((filter) => { const {key, label, disabled} = filter; diff --git a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx index 1767f6776db..4707778ad23 100644 --- a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx @@ -66,9 +66,26 @@ describe('', () => { expect(connectedFilterControl.find(Popover).exists()).toBe(false); }); - it('does render a button with a right action', () => { + it('always render a RightAction if forceShowMorefiltersButton is true', () => { const connectedFilterControl = mountWithAppProvider( - + + + , + ); + + expect(connectedFilterControl.find(Button).exists()).toBe(true); + }); + + it('renders a RightAction if forceShowMorefiltersButton is false and rightPopoverableActions do not fit on the "right action" container', () => { + const connectedFilterControl = mountWithAppProvider( + , ); diff --git a/src/components/Filters/tests/Filters.test.tsx b/src/components/Filters/tests/Filters.test.tsx index 6732a38e8f9..349e68a5b41 100644 --- a/src/components/Filters/tests/Filters.test.tsx +++ b/src/components/Filters/tests/Filters.test.tsx @@ -218,6 +218,46 @@ describe('', () => { ).toHaveLength(2); }); + it('forces showing the "More Filters" button if there are filters without shortcuts', () => { + const resourceFilters = mountWithAppProvider( + , + ); + + expect( + resourceFilters.find(ConnectedFilterControl).props() + .forceShowMorefiltersButton, + ).toBe(true); + }); + + it('does not force showing the "More Filters" button if all the filters have shorcuts', () => { + const mockPropsWithShortcuts: FiltersProps = { + onQueryChange: noop, + onQueryClear: noop, + onClearAll: noop, + filters: [ + { + key: 'filterOne', + label: 'Filter One', + filter: , + shortcut: true, + }, + { + key: 'filterTwo', + label: 'Filter Two', + filter: , + shortcut: true, + }, + ], + }; + const resourceFilters = mountWithAppProvider( + , + ); + expect( + resourceFilters.find(ConnectedFilterControl).props() + .forceShowMorefiltersButton, + ).toBe(false); + }); + it('receives shortcut filters with popoverOpen set to false on mount', () => { const resourceFilters = mountWithAppProvider( , diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index f521e4c6ef8..7889cdbe4a5 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,6 +150,11 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { + + // 'position' needs to sit below focus-ring since it will be overwritten + // with relative when the focus ring style is 'base' + // stylelint-disable-next-line order/properties-order + position: absolute; z-index: z-index(backdrop, $stacking-order); top: 0; right: 0; @@ -159,11 +164,6 @@ $stacking-order: ( border-radius: var(--p-border-radius-base); background-color: var(--p-surface); @include focus-ring($border-width: rem(1px)); - - // 'position' needs to sit below focus-ring since it will be overwritten - // with relative when the focus ring style is 'base' - // stylelint-disable-next-line order/properties-order - position: absolute; } &.error { From 26a002ab99c3715f991c2e954935b9cd16ae6fa8 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:00:21 -0400 Subject: [PATCH 06/25] adds UNRELEASED.md --- UNRELEASED.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index f50d13b618d..f4167961836 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -6,6 +6,8 @@ ### Bug fixes +- `Filters` only show the "More filters" button if necessary ([#](https://github.com/Shopify/polaris-react/pull/2856)). + ### Documentation ### Development workflow From 8680216f123e8f665b4f1e56be7585ce13bdda3f Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:33:21 -0400 Subject: [PATCH 07/25] solve conflicts --- .../ConnectedFilterControl/ConnectedFilterControl.scss | 7 ------- .../ConnectedFilterControl/ConnectedFilterControl.tsx | 4 ---- 2 files changed, 11 deletions(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index cac187df623..0f7795516fa 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -96,13 +96,6 @@ $stacking-order: ( } } -.RightContainerWithoutMoreFilters { - .Item > * > * { - border-top-right-radius: border-radius() !important; - border-bottom-right-radius: border-radius() !important; - } -} - .MoreFiltersButtonContainer * { border-top-left-radius: 0 !important; border-bottom-left-radius: 0 !important; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index 551c35ab22d..b8a15cdebaf 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -89,10 +89,6 @@ export class ConnectedFilterControl extends React.Component< newDesignLanguage && styles.newDesignLanguage, ); - const actionsToRender = - rightPopoverableActions && - this.getActionsToRender(rightPopoverableActions); - const shouldRenderMoreFiltersButton = forceShowMorefiltersButton || (rightPopoverableActions && From e184e9175813820efac737670ff935a711c6167b Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:48:25 -0400 Subject: [PATCH 08/25] fix linting --- src/components/Select/Select.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index 7889cdbe4a5..2ad51f0cc03 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,7 +150,6 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { - // 'position' needs to sit below focus-ring since it will be overwritten // with relative when the focus ring style is 'base' // stylelint-disable-next-line order/properties-order From bd58192fd2f3e6945b0f0f81d87a484c5451a248 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Wed, 8 Apr 2020 11:26:31 -0700 Subject: [PATCH 09/25] add two more tests --- .../tests/ConnectedFilterControl.test.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx index 4707778ad23..a04a49f5bec 100644 --- a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx @@ -93,6 +93,19 @@ describe('', () => { expect(connectedFilterControl.find(Button).exists()).toBe(true); }); + it('does not render a RightAction there are no actions hidden', () => { + const connectedFilterControl = mountWithAppProvider( + + + , + ); + + expect(connectedFilterControl.find(Button).exists()).toBe(false); + }); + it('does render a button with a popoverable action', () => { const connectedFilterControl = mountWithAppProvider( ', () => { expect(connectedFilterControl.find(Button)).toHaveLength(3); }); + it('hides an action if it does not fit', () => { + const connectedFilterControl = mountWithAppProvider( + + + , + ); + + connectedFilterControl.setState({availableWidth: 100}); + + expect(findActions(connectedFilterControl)).toHaveLength(2); + }); + it('renders auxiliary content', () => { const connectedFilterControl = mountWithAppProvider( }> @@ -188,3 +219,8 @@ function noop() {} function findById(wrapper: ReactWrapper, id: string) { return wrapper.find(`#${id}`).first(); } + +function findActions(wrapper: ReactWrapper) { + // this omits the invisible proxy actions used for measuring width + return wrapper.find('.Wrapper Button'); +} From 87eb592ad0623d6e66b89b1ba3358d9a1240890a Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Wed, 8 Apr 2020 13:31:34 -0700 Subject: [PATCH 10/25] refactor to remove branch which never executed --- .../ConnectedFilterControl.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index b8a15cdebaf..8c06d269612 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -92,7 +92,6 @@ export class ConnectedFilterControl extends React.Component< const shouldRenderMoreFiltersButton = forceShowMorefiltersButton || (rightPopoverableActions && - actionsToRender && rightPopoverableActions.length !== actionsToRender.length); const RightContainerClassName = classNames( @@ -100,11 +99,15 @@ export class ConnectedFilterControl extends React.Component< !shouldRenderMoreFiltersButton && styles.RightContainerWithoutMoreFilters, ); - const rightMarkup = actionsToRender ? ( -
- {this.popoverFrom(actionsToRender)} -
- ) : null; + const rightMarkup = + actionsToRender.length > 0 ? ( +
+ {this.popoverFrom(actionsToRender)} +
+ ) : null; const moreFiltersButtonContainerClassname = classNames( styles.MoreFiltersButtonContainer, From c953360fd2500d4cf5e38aaecdc7ed70fc10fe3a Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 18 Mar 2020 11:59:54 -0400 Subject: [PATCH 11/25] adds a hideMoreFiltersButton prop to the Filters component --- src/components/Filters/Filters.tsx | 4 ++++ .../ConnectedFilterControl.tsx | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 62bef8411c2..3be58c917c6 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -87,6 +87,8 @@ export interface FiltersProps { helpText?: string | React.ReactNode; /** Hide tags for applied filters */ hideTags?: boolean; + /** Hide "More filters" button **/ + hideMoreFiltersButton?: boolean; } type ComposedProps = FiltersProps & @@ -135,6 +137,7 @@ class FiltersInner extends React.Component { helpText, hideTags, newDesignLanguage, + hideMoreFiltersButton, } = this.props; const {resourceName} = this.context; const {open, readyForFocus} = this.state; @@ -249,6 +252,7 @@ class FiltersInner extends React.Component { rightAction={rightActionMarkup} auxiliary={children} disabled={disabled} + hideMoreFiltersButton={hideMoreFiltersButton} > 0 && + hideMoreFiltersButton ? null : ( + {rightAction} + ); + const rightMarkup = rightPopoverableActions ? (
{this.popoverFrom(actionsToRender)} @@ -105,7 +116,7 @@ export class ConnectedFilterControl extends React.Component< ref={this.moreFiltersButtonContainer} className={moreFiltersButtonContainerClassname} > - {rightAction} + {moreFilterButton}
) : null; From 0ffdfbe2340880ae1db5ce3a2a76c69c2ae6244b Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 18 Mar 2020 17:09:53 -0400 Subject: [PATCH 12/25] removes the hideMoreFiltersButton prop --- src/components/Filters/Filters.tsx | 15 +++++------- .../ConnectedFilterControl.tsx | 23 ++++++++++--------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 3be58c917c6..99379d59faf 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -87,8 +87,6 @@ export interface FiltersProps { helpText?: string | React.ReactNode; /** Hide tags for applied filters */ hideTags?: boolean; - /** Hide "More filters" button **/ - hideMoreFiltersButton?: boolean; } type ComposedProps = FiltersProps & @@ -137,7 +135,6 @@ class FiltersInner extends React.Component { helpText, hideTags, newDesignLanguage, - hideMoreFiltersButton, } = this.props; const {resourceName} = this.context; const {open, readyForFocus} = this.state; @@ -246,13 +243,15 @@ class FiltersInner extends React.Component { plural: intl.translate('Polaris.ResourceList.defaultItemPlural'), }; + const transformedFilters = this.transformFilters(filters); + const filtersControlMarkup = ( transformedFilters.length} > { } } - private transformFilters( - filters: FilterInterface[], - ): ConnectedFilterControlProps['rightPopoverableActions'] | null { - const transformedActions: ConnectedFilterControlProps['rightPopoverableActions'] = []; + private transformFilters(filters: FilterInterface[]) { + const transformedActions: Required = []; getShortcutFilters(filters).forEach((filter) => { const {key, label, disabled} = filter; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index bf70e04d116..325be3349da 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -25,7 +25,7 @@ export interface ConnectedFilterControlProps { rightAction?: React.ReactNode; auxiliary?: React.ReactNode; disabled?: boolean; - hideMoreFiltersButton?: boolean; + forceShowMorefiltersButton?: boolean; } interface ComputedProperty { @@ -75,7 +75,7 @@ export class ConnectedFilterControl extends React.Component< rightPopoverableActions, rightAction, auxiliary, - hideMoreFiltersButton, + forceShowMorefiltersButton = true, } = this.props; const actionsToRender = @@ -89,16 +89,17 @@ export class ConnectedFilterControl extends React.Component< newDesignLanguage && styles.newDesignLanguage, ); - // If there's only one action, and it fits we don't show the "More filters" button - const moreFilterButton = + const actionsToRender = rightPopoverableActions && - rightPopoverableActions.length === 1 && - this.getActionsToRender(rightPopoverableActions).length > 0 && - hideMoreFiltersButton ? null : ( - {rightAction} - ); + this.getActionsToRender(rightPopoverableActions); + + const shouldRenderMoreFiltersButton = + forceShowMorefiltersButton || + (rightPopoverableActions && + actionsToRender && + rightPopoverableActions.length !== actionsToRender.length); - const rightMarkup = rightPopoverableActions ? ( + const rightMarkup = actionsToRender ? (
{this.popoverFrom(actionsToRender)}
@@ -116,7 +117,7 @@ export class ConnectedFilterControl extends React.Component< ref={this.moreFiltersButtonContainer} className={moreFiltersButtonContainerClassname} > - {moreFilterButton} + {shouldRenderMoreFiltersButton && {rightAction}}
) : null; From eef8cb9f93525fa6362cfac144d6ee9c35053366 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Fri, 27 Mar 2020 11:42:42 -0400 Subject: [PATCH 13/25] address Tim Layton comments --- .../ConnectedFilterControl.scss | 7 +++++++ .../ConnectedFilterControl/ConnectedFilterControl.tsx | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index 2bf617701c8..8675bb2eee7 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -83,6 +83,13 @@ $stacking-order: ( } } +.RightContainerWithoutMoreFilters { + .Item > * > * { + border-top-right-radius: border-radius() !important; + border-bottom-right-radius: border-radius() !important; + } +} + .MoreFiltersButtonContainer * { border-top-left-radius: 0 !important; border-bottom-left-radius: 0 !important; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index 325be3349da..551c35ab22d 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -99,8 +99,13 @@ export class ConnectedFilterControl extends React.Component< actionsToRender && rightPopoverableActions.length !== actionsToRender.length); + const RightContainerClassName = classNames( + styles.RightContainer, + !shouldRenderMoreFiltersButton && styles.RightContainerWithoutMoreFilters, + ); + const rightMarkup = actionsToRender ? ( -
+
{this.popoverFrom(actionsToRender)}
) : null; @@ -209,6 +214,10 @@ export class ConnectedFilterControl extends React.Component< if (actionWidth <= remainingWidth) { actionsToReturn.push(action); remainingWidth -= actionWidth; + } else { + // When we can't fit an action, we break the loop. + // The ones that didn't fit will be accessible through the "More filters" button + break; } } return actionsToReturn; From 71c7b49e44bcbbf922fd866dfcc180e54749c395 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Tue, 31 Mar 2020 10:29:02 -0400 Subject: [PATCH 14/25] address border-radius problem --- .../ConnectedFilterControl.scss | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index 8675bb2eee7..cac187df623 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -76,6 +76,19 @@ $stacking-order: ( } } +.RightContainerWithoutMoreFilters { + .Item:last-child > * > * { + border-top-right-radius: var( + --p-border-radius-base, + border-radius() + ) !important; + border-bottom-right-radius: var( + --p-border-radius-base, + border-radius() + ) !important; + } +} + .newDesignLanguage .RightContainer { .Item:first-of-type > * > * { border-top-left-radius: var(--p-border-radius-base) !important; From a9aa5089aef74ed4f80f12c58cc595dd2eddff64 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 11:50:04 -0400 Subject: [PATCH 15/25] adds tests and formats --- src/components/Filters/Filters.tsx | 4 +- .../tests/ConnectedFilterControl.test.tsx | 21 +++++++++- src/components/Filters/tests/Filters.test.tsx | 40 +++++++++++++++++++ src/components/Select/Select.scss | 10 ++--- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/components/Filters/Filters.tsx b/src/components/Filters/Filters.tsx index 99379d59faf..3f08fb817b8 100644 --- a/src/components/Filters/Filters.tsx +++ b/src/components/Filters/Filters.tsx @@ -517,7 +517,9 @@ class FiltersInner extends React.Component { } private transformFilters(filters: FilterInterface[]) { - const transformedActions: Required = []; + const transformedActions: Required< + ConnectedFilterControlProps['rightPopoverableActions'] + > = []; getShortcutFilters(filters).forEach((filter) => { const {key, label, disabled} = filter; diff --git a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx index 1767f6776db..4707778ad23 100644 --- a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx @@ -66,9 +66,26 @@ describe('', () => { expect(connectedFilterControl.find(Popover).exists()).toBe(false); }); - it('does render a button with a right action', () => { + it('always render a RightAction if forceShowMorefiltersButton is true', () => { const connectedFilterControl = mountWithAppProvider( - + + + , + ); + + expect(connectedFilterControl.find(Button).exists()).toBe(true); + }); + + it('renders a RightAction if forceShowMorefiltersButton is false and rightPopoverableActions do not fit on the "right action" container', () => { + const connectedFilterControl = mountWithAppProvider( + , ); diff --git a/src/components/Filters/tests/Filters.test.tsx b/src/components/Filters/tests/Filters.test.tsx index 6732a38e8f9..349e68a5b41 100644 --- a/src/components/Filters/tests/Filters.test.tsx +++ b/src/components/Filters/tests/Filters.test.tsx @@ -218,6 +218,46 @@ describe('', () => { ).toHaveLength(2); }); + it('forces showing the "More Filters" button if there are filters without shortcuts', () => { + const resourceFilters = mountWithAppProvider( + , + ); + + expect( + resourceFilters.find(ConnectedFilterControl).props() + .forceShowMorefiltersButton, + ).toBe(true); + }); + + it('does not force showing the "More Filters" button if all the filters have shorcuts', () => { + const mockPropsWithShortcuts: FiltersProps = { + onQueryChange: noop, + onQueryClear: noop, + onClearAll: noop, + filters: [ + { + key: 'filterOne', + label: 'Filter One', + filter: , + shortcut: true, + }, + { + key: 'filterTwo', + label: 'Filter Two', + filter: , + shortcut: true, + }, + ], + }; + const resourceFilters = mountWithAppProvider( + , + ); + expect( + resourceFilters.find(ConnectedFilterControl).props() + .forceShowMorefiltersButton, + ).toBe(false); + }); + it('receives shortcut filters with popoverOpen set to false on mount', () => { const resourceFilters = mountWithAppProvider( , diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index f521e4c6ef8..7889cdbe4a5 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,6 +150,11 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { + + // 'position' needs to sit below focus-ring since it will be overwritten + // with relative when the focus ring style is 'base' + // stylelint-disable-next-line order/properties-order + position: absolute; z-index: z-index(backdrop, $stacking-order); top: 0; right: 0; @@ -159,11 +164,6 @@ $stacking-order: ( border-radius: var(--p-border-radius-base); background-color: var(--p-surface); @include focus-ring($border-width: rem(1px)); - - // 'position' needs to sit below focus-ring since it will be overwritten - // with relative when the focus ring style is 'base' - // stylelint-disable-next-line order/properties-order - position: absolute; } &.error { From c1908e05c7159d4557b17bfc9be50bb87ca5594e Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:00:21 -0400 Subject: [PATCH 16/25] adds UNRELEASED.md --- UNRELEASED.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 33afb252840..55b9665b0e6 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -13,6 +13,8 @@ ### Bug fixes +- `Filters` only show the "More filters" button if necessary ([#](https://github.com/Shopify/polaris-react/pull/2856)). + ### Documentation ### Development workflow From ed93329c163bd0fa86fd927657fdcffa56651327 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:33:21 -0400 Subject: [PATCH 17/25] solve conflicts --- .../ConnectedFilterControl/ConnectedFilterControl.scss | 7 ------- .../ConnectedFilterControl/ConnectedFilterControl.tsx | 4 ---- 2 files changed, 11 deletions(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss index cac187df623..0f7795516fa 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss @@ -96,13 +96,6 @@ $stacking-order: ( } } -.RightContainerWithoutMoreFilters { - .Item > * > * { - border-top-right-radius: border-radius() !important; - border-bottom-right-radius: border-radius() !important; - } -} - .MoreFiltersButtonContainer * { border-top-left-radius: 0 !important; border-bottom-left-radius: 0 !important; diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index 551c35ab22d..b8a15cdebaf 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -89,10 +89,6 @@ export class ConnectedFilterControl extends React.Component< newDesignLanguage && styles.newDesignLanguage, ); - const actionsToRender = - rightPopoverableActions && - this.getActionsToRender(rightPopoverableActions); - const shouldRenderMoreFiltersButton = forceShowMorefiltersButton || (rightPopoverableActions && From 39f859763850859f82c420eac8e34581f54f40f9 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 12:48:25 -0400 Subject: [PATCH 18/25] fix linting --- src/components/Select/Select.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index 7889cdbe4a5..2ad51f0cc03 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,7 +150,6 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { - // 'position' needs to sit below focus-ring since it will be overwritten // with relative when the focus ring style is 'base' // stylelint-disable-next-line order/properties-order From 993aed7c3d08afcd17faada967c1352a11c26600 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Wed, 8 Apr 2020 11:26:31 -0700 Subject: [PATCH 19/25] add two more tests --- .../tests/ConnectedFilterControl.test.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx index 4707778ad23..a04a49f5bec 100644 --- a/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx @@ -93,6 +93,19 @@ describe('', () => { expect(connectedFilterControl.find(Button).exists()).toBe(true); }); + it('does not render a RightAction there are no actions hidden', () => { + const connectedFilterControl = mountWithAppProvider( + + + , + ); + + expect(connectedFilterControl.find(Button).exists()).toBe(false); + }); + it('does render a button with a popoverable action', () => { const connectedFilterControl = mountWithAppProvider( ', () => { expect(connectedFilterControl.find(Button)).toHaveLength(3); }); + it('hides an action if it does not fit', () => { + const connectedFilterControl = mountWithAppProvider( + + + , + ); + + connectedFilterControl.setState({availableWidth: 100}); + + expect(findActions(connectedFilterControl)).toHaveLength(2); + }); + it('renders auxiliary content', () => { const connectedFilterControl = mountWithAppProvider( }> @@ -188,3 +219,8 @@ function noop() {} function findById(wrapper: ReactWrapper, id: string) { return wrapper.find(`#${id}`).first(); } + +function findActions(wrapper: ReactWrapper) { + // this omits the invisible proxy actions used for measuring width + return wrapper.find('.Wrapper Button'); +} From c28ad565ab46382d55d986d4b496f74b0f927ae5 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Wed, 8 Apr 2020 13:31:34 -0700 Subject: [PATCH 20/25] refactor to remove branch which never executed --- .../ConnectedFilterControl.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx index b8a15cdebaf..8c06d269612 100644 --- a/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx +++ b/src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx @@ -92,7 +92,6 @@ export class ConnectedFilterControl extends React.Component< const shouldRenderMoreFiltersButton = forceShowMorefiltersButton || (rightPopoverableActions && - actionsToRender && rightPopoverableActions.length !== actionsToRender.length); const RightContainerClassName = classNames( @@ -100,11 +99,15 @@ export class ConnectedFilterControl extends React.Component< !shouldRenderMoreFiltersButton && styles.RightContainerWithoutMoreFilters, ); - const rightMarkup = actionsToRender ? ( -
- {this.popoverFrom(actionsToRender)} -
- ) : null; + const rightMarkup = + actionsToRender.length > 0 ? ( +
+ {this.popoverFrom(actionsToRender)} +
+ ) : null; const moreFiltersButtonContainerClassname = classNames( styles.MoreFiltersButtonContainer, From 06945aa32cc6e9163a3a0425bdc37837c8aab1f4 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Wed, 8 Apr 2020 16:53:30 -0400 Subject: [PATCH 21/25] update UNRELEASED.md and revert Select.scss change --- UNRELEASED.md | 3 ++- src/components/Select/Select.scss | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 55b9665b0e6..cd870a333d9 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -12,8 +12,9 @@ - Added utilities for parsing video duration (https://polaris.shopify.com/components/images-and-icons/video-thumbnail) ([#2725](https://github.com/Shopify/polaris-react/pull/2725)) ### Bug fixes +- Updated `Filters` to only show the "More filters" button if necessary ([#2856](https://github.com/Shopify/polaris-react/pull/2856)). -- `Filters` only show the "More filters" button if necessary ([#](https://github.com/Shopify/polaris-react/pull/2856)). +### Bug fixes ### Documentation diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index 2ad51f0cc03..f521e4c6ef8 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,10 +150,6 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { - // 'position' needs to sit below focus-ring since it will be overwritten - // with relative when the focus ring style is 'base' - // stylelint-disable-next-line order/properties-order - position: absolute; z-index: z-index(backdrop, $stacking-order); top: 0; right: 0; @@ -163,6 +159,11 @@ $stacking-order: ( border-radius: var(--p-border-radius-base); background-color: var(--p-surface); @include focus-ring($border-width: rem(1px)); + + // 'position' needs to sit below focus-ring since it will be overwritten + // with relative when the focus ring style is 'base' + // stylelint-disable-next-line order/properties-order + position: absolute; } &.error { From bfa40a0a5a4632a9e09d11f3f6d28d8522f83e52 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Wed, 8 Apr 2020 14:59:21 -0700 Subject: [PATCH 22/25] markdown format --- UNRELEASED.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index b5f84c35e34..644347876b3 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -10,8 +10,6 @@ ### Enhancements - Added utilities for parsing video duration (https://polaris.shopify.com/components/images-and-icons/video-thumbnail) ([#2725](https://github.com/Shopify/polaris-react/pull/2725)) - -### Bug fixes - Updated `Filters` to only show the "More filters" button if necessary ([#2856](https://github.com/Shopify/polaris-react/pull/2856)). ### Bug fixes From 8b595f2772aaad0bffd4075b5a0692689a7b2011 Mon Sep 17 00:00:00 2001 From: Ricardo Macario Date: Thu, 9 Apr 2020 10:42:48 -0400 Subject: [PATCH 23/25] revert again Select.scss change --- src/components/Select/Select.scss | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/Select/Select.scss b/src/components/Select/Select.scss index 2ad51f0cc03..f521e4c6ef8 100644 --- a/src/components/Select/Select.scss +++ b/src/components/Select/Select.scss @@ -150,10 +150,6 @@ $stacking-order: ( .newDesignLanguage { .Backdrop { - // 'position' needs to sit below focus-ring since it will be overwritten - // with relative when the focus ring style is 'base' - // stylelint-disable-next-line order/properties-order - position: absolute; z-index: z-index(backdrop, $stacking-order); top: 0; right: 0; @@ -163,6 +159,11 @@ $stacking-order: ( border-radius: var(--p-border-radius-base); background-color: var(--p-surface); @include focus-ring($border-width: rem(1px)); + + // 'position' needs to sit below focus-ring since it will be overwritten + // with relative when the focus ring style is 'base' + // stylelint-disable-next-line order/properties-order + position: absolute; } &.error { From aa990c4857fd7b713d98278364dce42f679e4208 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Thu, 9 Apr 2020 12:18:50 -0700 Subject: [PATCH 24/25] replace bug fixes header --- UNRELEASED.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 85591d0bd73..3bdefd5e802 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -8,6 +8,8 @@ - Updated `Filters` to only show the "More filters" button if necessary ([#2856](https://github.com/Shopify/polaris-react/pull/2856)). +### Bug fixes + ### Documentation ### Development workflow From afb500a3684c5d9786afe425c2f5a836bb6c1999 Mon Sep 17 00:00:00 2001 From: Tim Layton Date: Thu, 9 Apr 2020 12:46:15 -0700 Subject: [PATCH 25/25] one snuck by --- src/components/MediaCard/MediaCard.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/MediaCard/MediaCard.tsx b/src/components/MediaCard/MediaCard.tsx index e60ec8ad674..827acc5113b 100644 --- a/src/components/MediaCard/MediaCard.tsx +++ b/src/components/MediaCard/MediaCard.tsx @@ -4,7 +4,7 @@ import {HorizontalDotsMinor} from '@shopify/polaris-icons'; import {useToggle} from '../../utilities/use-toggle'; import {classNames} from '../../utilities/css'; import {useI18n} from '../../utilities/i18n'; -import {Action, ActionListItemDescriptor} from '../../types'; +import type {Action, ActionListItemDescriptor} from '../../types'; import {Card} from '../Card'; import {Button, buttonFrom} from '../Button'; import {Heading} from '../Heading';