From fdf4afc793f786071ab390e886619533533a5782 Mon Sep 17 00:00:00 2001 From: Thomas Tau Date: Fri, 9 Feb 2024 20:49:43 +0000 Subject: [PATCH 1/4] Add active prop to ResourceItem --- polaris-react/playground/Playground.tsx | 80 ++++++++++++++++++- .../ResourceItem/ResourceItem.module.scss | 8 ++ .../components/ResourceItem/ResourceItem.tsx | 11 ++- .../ResourceItem/tests/ResourceItem.test.tsx | 48 +++++++++++ 4 files changed, 141 insertions(+), 6 deletions(-) diff --git a/polaris-react/playground/Playground.tsx b/polaris-react/playground/Playground.tsx index 355f6b984f2..d2a0a4a4f54 100644 --- a/polaris-react/playground/Playground.tsx +++ b/polaris-react/playground/Playground.tsx @@ -1,11 +1,85 @@ -import React from 'react'; +import React, {useState} from 'react'; +import type {ResourceListProps} from '@shopify/polaris'; -import {Page} from '../src'; +import { + Page, + LegacyCard, + ResourceList, + ResourceItem, + Text, + Avatar, +} from '../src'; export function Playground() { + const [selectedItems, setSelectedItems] = useState< + ResourceListProps['selectedItems'] + >([]); + return ( - {/* Add the code you want to test in here */} + + { + const {id, url, avatarSource, name, location} = item; + + return ( + + } + accessibilityLabel={`View details for ${name}`} + name={name} + onClick={() => console.log('clicked')} + > +

+ + {name} + +

+
{location}
+ + ); + }} + /> + ); } diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.module.scss b/polaris-react/src/components/ResourceItem/ResourceItem.module.scss index fcbaafee50e..762df5d70d0 100644 --- a/polaris-react/src/components/ResourceItem/ResourceItem.module.scss +++ b/polaris-react/src/components/ResourceItem/ResourceItem.module.scss @@ -84,6 +84,14 @@ margin-right: 0; } +.active { + cursor: default; + + &:hover { + background-color: transparent; + } +} + // Item actions .Actions { > * { diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.tsx b/polaris-react/src/components/ResourceItem/ResourceItem.tsx index 7e9e47ee6de..0443d9946e3 100644 --- a/polaris-react/src/components/ResourceItem/ResourceItem.tsx +++ b/polaris-react/src/components/ResourceItem/ResourceItem.tsx @@ -30,6 +30,8 @@ import styles from './ResourceItem.module.scss'; type Alignment = 'leading' | 'trailing' | 'center' | 'fill' | 'baseline'; interface BaseProps { + /** Whether or not the current item is the active */ + active?: boolean; /** Visually hidden text for screen readers used for item link */ accessibilityLabel?: string; /** Individual item name used by various text labels */ @@ -159,6 +161,7 @@ class BaseResourceItem extends Component { dataHref, breakpoints, onMouseOver, + active, } = this.props; const {actionsMenuVisible, focused, focusedInner, selected} = this.state; @@ -219,6 +222,7 @@ class BaseResourceItem extends Component { selectMode && styles.selectMode, persistActions && styles.persistActions, focusedInner && styles.focusedInner, + active && styles.active, ); const listItemClassName = classNames( @@ -351,7 +355,7 @@ class BaseResourceItem extends Component {
{} : this.handleClick} onFocus={this.handleFocus} onBlur={this.handleBlur} onKeyUp={this.handleKeyUp} @@ -359,7 +363,7 @@ class BaseResourceItem extends Component { onMouseOut={this.handleMouseOut} data-href={url} > - {accessibleMarkup} + {active ? null : accessibleMarkup} {containerMarkup}
@@ -458,12 +462,13 @@ class BaseResourceItem extends Component { // This fires onClick when there is a URL on the item private handleKeyUp = (event: React.KeyboardEvent) => { const { + active, onClick = noop, context: {selectMode}, } = this.props; const {key} = event; - if (key === 'Enter' && this.props.url && !selectMode) { + if (key === 'Enter' && this.props.url && !selectMode && !active) { onClick(); } }; diff --git a/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx b/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx index c04320590b4..18ac29071c8 100644 --- a/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx +++ b/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx @@ -227,6 +227,22 @@ describe('', () => { expect(element).toContainReactComponent('div', {'data-href': url} as any); }); + + it('does not render an when active prop is true', () => { + const element = mountWithApp( + + + , + ); + + expect(element).not.toContainReactComponent(UnstyledLink); + }); }); describe('external', () => { @@ -388,6 +404,38 @@ describe('', () => { expect(onClick).not.toHaveBeenCalled(); }); + it('does not call onClick when hitting keyUp on the item when onClick exists, url exists and is not active', () => { + const onClick = jest.fn(); + const wrapper = mountWithApp( + + + , + ); + + findResourceItem(wrapper)!.trigger('onKeyUp', {key: 'Enter'}); + expect(onClick).not.toHaveBeenCalled(); + }); + + it('does not call onClick when clicking on the item when onClick exists and is active', () => { + const onClick = jest.fn(); + const wrapper = mountWithApp( + + + , + ); + + findResourceItem(wrapper)!.trigger('onClick', { + stopPropagation: () => {}, + nativeEvent: {}, + }); + expect(onClick).not.toHaveBeenCalledWith(itemId); + }); + it('calls window.open on ctrlKey + click', () => { const wrapper = mountWithApp( From 6b93b862da9d6e1f5c118f98a3c45dede33396ec Mon Sep 17 00:00:00 2001 From: Thomas Tau Date: Fri, 9 Feb 2024 21:23:38 +0000 Subject: [PATCH 2/4] revert playground file --- .changeset/proud-items-do.md | 5 ++ polaris-react/playground/Playground.tsx | 80 +------------------------ 2 files changed, 8 insertions(+), 77 deletions(-) create mode 100644 .changeset/proud-items-do.md diff --git a/.changeset/proud-items-do.md b/.changeset/proud-items-do.md new file mode 100644 index 00000000000..2360f757b24 --- /dev/null +++ b/.changeset/proud-items-do.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': minor +--- + +Add a new active prop in ResourceItem component diff --git a/polaris-react/playground/Playground.tsx b/polaris-react/playground/Playground.tsx index d2a0a4a4f54..355f6b984f2 100644 --- a/polaris-react/playground/Playground.tsx +++ b/polaris-react/playground/Playground.tsx @@ -1,85 +1,11 @@ -import React, {useState} from 'react'; -import type {ResourceListProps} from '@shopify/polaris'; +import React from 'react'; -import { - Page, - LegacyCard, - ResourceList, - ResourceItem, - Text, - Avatar, -} from '../src'; +import {Page} from '../src'; export function Playground() { - const [selectedItems, setSelectedItems] = useState< - ResourceListProps['selectedItems'] - >([]); - return ( - - { - const {id, url, avatarSource, name, location} = item; - - return ( - - } - accessibilityLabel={`View details for ${name}`} - name={name} - onClick={() => console.log('clicked')} - > -

- - {name} - -

-
{location}
- - ); - }} - /> - + {/* Add the code you want to test in here */} ); } From 9ecb59450dc654a382d964db6375b0bf41bd33c5 Mon Sep 17 00:00:00 2001 From: Thomas Tau Date: Tue, 13 Feb 2024 20:20:29 +0000 Subject: [PATCH 3/4] Rename active by disabled prop name + add disabled state on checkbox when selectable is true --- .../ResourceItem/ResourceItem.module.scss | 3 ++- .../components/ResourceItem/ResourceItem.tsx | 18 ++++++++-------- .../ResourceItem/tests/ResourceItem.test.tsx | 21 +++++++++++++------ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.module.scss b/polaris-react/src/components/ResourceItem/ResourceItem.module.scss index 762df5d70d0..d2eb1fb3e88 100644 --- a/polaris-react/src/components/ResourceItem/ResourceItem.module.scss +++ b/polaris-react/src/components/ResourceItem/ResourceItem.module.scss @@ -84,8 +84,9 @@ margin-right: 0; } -.active { +.disabled { cursor: default; + color: var(--p-color-text-secondary); &:hover { background-color: transparent; diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.tsx b/polaris-react/src/components/ResourceItem/ResourceItem.tsx index 0443d9946e3..e8a97112398 100644 --- a/polaris-react/src/components/ResourceItem/ResourceItem.tsx +++ b/polaris-react/src/components/ResourceItem/ResourceItem.tsx @@ -30,8 +30,8 @@ import styles from './ResourceItem.module.scss'; type Alignment = 'leading' | 'trailing' | 'center' | 'fill' | 'baseline'; interface BaseProps { - /** Whether or not the current item is the active */ - active?: boolean; + /** Whether or not interaction is disabled */ + disabled?: boolean; /** Visually hidden text for screen readers used for item link */ accessibilityLabel?: string; /** Individual item name used by various text labels */ @@ -161,7 +161,7 @@ class BaseResourceItem extends Component { dataHref, breakpoints, onMouseOver, - active, + disabled, } = this.props; const {actionsMenuVisible, focused, focusedInner, selected} = this.state; @@ -186,7 +186,7 @@ class BaseResourceItem extends Component { label={checkboxAccessibilityLabel} labelHidden checked={selected} - disabled={loading} + disabled={loading || disabled} bleedInlineStart="300" bleedInlineEnd="300" bleedBlockStart="300" @@ -222,7 +222,7 @@ class BaseResourceItem extends Component { selectMode && styles.selectMode, persistActions && styles.persistActions, focusedInner && styles.focusedInner, - active && styles.active, + disabled && styles.disabled, ); const listItemClassName = classNames( @@ -355,7 +355,7 @@ class BaseResourceItem extends Component {
{} : this.handleClick} + onClick={disabled ? () => {} : this.handleClick} onFocus={this.handleFocus} onBlur={this.handleBlur} onKeyUp={this.handleKeyUp} @@ -363,7 +363,7 @@ class BaseResourceItem extends Component { onMouseOut={this.handleMouseOut} data-href={url} > - {active ? null : accessibleMarkup} + {disabled ? null : accessibleMarkup} {containerMarkup}
@@ -462,13 +462,13 @@ class BaseResourceItem extends Component { // This fires onClick when there is a URL on the item private handleKeyUp = (event: React.KeyboardEvent) => { const { - active, + disabled, onClick = noop, context: {selectMode}, } = this.props; const {key} = event; - if (key === 'Enter' && this.props.url && !selectMode && !active) { + if (key === 'Enter' && this.props.url && !selectMode && !disabled) { onClick(); } }; diff --git a/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx b/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx index 18ac29071c8..70bc7cc05c0 100644 --- a/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx +++ b/polaris-react/src/components/ResourceItem/tests/ResourceItem.test.tsx @@ -228,7 +228,7 @@ describe('', () => { expect(element).toContainReactComponent('div', {'data-href': url} as any); }); - it('does not render an when active prop is true', () => { + it('does not render an when disabled prop is true', () => { const element = mountWithApp( ', () => { url={url} onClick={noop} accessibilityLabel={ariaLabel} - active + disabled /> , ); @@ -404,11 +404,11 @@ describe('', () => { expect(onClick).not.toHaveBeenCalled(); }); - it('does not call onClick when hitting keyUp on the item when onClick exists, url exists and is not active', () => { + it('does not call onClick when hitting keyUp on the item when onClick exists, url exists and is disabled', () => { const onClick = jest.fn(); const wrapper = mountWithApp( - + , ); @@ -416,7 +416,7 @@ describe('', () => { expect(onClick).not.toHaveBeenCalled(); }); - it('does not call onClick when clicking on the item when onClick exists and is active', () => { + it('does not call onClick when clicking on the item when onClick exists and is disabled', () => { const onClick = jest.fn(); const wrapper = mountWithApp( @@ -424,7 +424,7 @@ describe('', () => { id={itemId} onClick={onClick} accessibilityLabel={ariaLabel} - active + disabled /> , ); @@ -496,6 +496,15 @@ describe('', () => { false, ); }); + + it('renders a disabled Checkbox if the item is disabled', () => { + const wrapper = mountWithApp( + + + , + ); + expect(wrapper).toContainReactComponent(Checkbox, {disabled: true}); + }); }); describe('SelectMode', () => { From e6c42f1f919c929d6830a3987288190617d6f5f2 Mon Sep 17 00:00:00 2001 From: Thomas Tau Date: Tue, 13 Feb 2024 23:42:56 +0000 Subject: [PATCH 4/4] Change changelog description + Add new story --- .changeset/proud-items-do.md | 2 +- .../ResourceItem/ResourceItem.stories.tsx | 66 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/.changeset/proud-items-do.md b/.changeset/proud-items-do.md index 2360f757b24..e5cc17ae927 100644 --- a/.changeset/proud-items-do.md +++ b/.changeset/proud-items-do.md @@ -2,4 +2,4 @@ '@shopify/polaris': minor --- -Add a new active prop in ResourceItem component +Added a `disabled` prop to `ResourceItem` diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx b/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx index 8d15bc2683c..7d2b6263d60 100644 --- a/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx +++ b/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx @@ -301,3 +301,69 @@ export function WithVerticalAlignment() { ); } + +export function WithDisabledState() { + const [selectedItems, setSelectedItems] = useState< + ResourceListProps['selectedItems'] + >([]); + + return ( + + { + const {id, url, avatarSource, name, location} = item; + + return ( + + } + accessibilityLabel={`View details for ${name}`} + name={name} + > +

+ + {name} + +

+
{location}
+ + ); + }} + /> + + ); +}