Skip to content

Commit f207706

Browse files
laurkimsophschneider
authored andcommitted
[ResourceItem] Fix broken focus ring styles (#10268)
### WHY are these changes introduced? Resolves #10175. Focus ring was broken on ResourceItem prior to uplift and was flagged during se23 clean up. ### WHAT is this pull request doing? Fixes minor issue on ResourceList.stories.tsx where LegacyCard was not using the correct border radius on xs screens (should match IndexTable where border-radius is set to `0` for xs screens). Fixes focus ring styles by opting to use `offset` property as opposed to the `focus-ring` mixin. <details> <summary>ResourceItem focus state — before</summary> <img src="https://github.com/Shopify/polaris/assets/26749317/0f2fc337-34d5-483d-b62b-df907277ff22" alt="ResourceItem focus state — before"> </details> <details> <summary>ResourceItem focus state — after</summary> <img src="https://github.com/Shopify/polaris/assets/26749317/383eb27e-0c2b-40d4-ab90-52205312ce73" alt="ResourceItem focus state — after"> </details> ### How to 🎩 [ResourceItem Storybook](https://5d559397bae39100201eedc1-egkosgfazl.chromatic.com/?path=/story/all-components-resourceitem--default) [ResourceItem Prod Storybook](https://storybook.polaris.shopify.com/?path=/story/all-components-resourceitem--default&globals=polarisSummerEditions2023:true) [BulkActions Storybook](https://5d559397bae39100201eedc1-egkosgfazl.chromatic.com/?path=/story/all-components-resourcelist--with-bulk-actions) [BulkActions Prod Storybook](https://storybook.polaris.shopify.com/?path=/story/all-components-resourcelist--with-bulk-actions&globals=polarisSummerEditions2023:true) 🖥 [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) ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] 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
1 parent d963e84 commit f207706

File tree

5 files changed

+89
-43
lines changed

5 files changed

+89
-43
lines changed

.changeset/light-seahorses-sit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': patch
3+
---
4+
5+
Fixed broken focus ring styles on `ResourceItem` component

polaris-react/src/components/ResourceItem/ResourceItem.scss

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
}
112112

113113
.ListItem {
114-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
114+
// stylelint-disable-next-line -- include focus-ring mixin to prevent jump in content for first focused element
115115
@include focus-ring($border-width: -1px);
116116

117117
.ListItem + & {
@@ -120,38 +120,61 @@
120120
@include item-separator;
121121
}
122122

123-
&:not(.hasBulkActions):not(.selectMode) {
124-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
125-
&::after {
126-
border-radius: var(--p-border-radius-05);
127-
}
123+
&.focused {
124+
// stylelint-disable-next-line -- remove focus-ring mixin to use outline styles
125+
@include no-focus-ring;
126+
outline: var(--p-border-width-2) solid
127+
var(--p-color-border-interactive-focus);
128+
outline-offset: calc(-1 * var(--p-space-05));
129+
// stylelint-disable-next-line -- custom property
130+
z-index: var(--pc-resource-item-clickable-stacking-order);
131+
border-radius: var(--p-border-radius-0-experimental);
128132

129-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
130-
&:last-of-type {
131-
border-bottom-left-radius: var(--p-border-radius-2);
132-
border-bottom-right-radius: var(--p-border-radius-2);
133-
// stylelint-disable selector-max-specificity -- Needed due to the nesting of the elements that change border-radius based on selection mode
134-
.ItemWrapper {
135-
border-radius: inherit;
133+
@media #{$p-breakpoints-sm-up} {
134+
border-radius: var(--p-border-radius-3);
135+
136+
&:first-of-type {
137+
border-bottom-left-radius: var(--p-border-radius-0-experimental);
138+
border-bottom-right-radius: var(--p-border-radius-0-experimental);
136139
}
137140

138-
&.focused::after {
139-
border-bottom-left-radius: var(--p-border-radius-2);
140-
border-bottom-right-radius: var(--p-border-radius-2);
141+
&:last-of-type {
142+
border-top-left-radius: var(--p-border-radius-0-experimental);
143+
border-top-right-radius: var(--p-border-radius-0-experimental);
141144
}
142-
// stylelint-enable
143145
}
144-
}
145146

146-
&.focused {
147-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
148-
@include focus-ring($style: 'focused');
149-
// stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
150-
z-index: var(--pc-resource-item-clickable-stacking-order);
151-
}
147+
&:only-child {
148+
border-radius: var(--p-border-radius-0-experimental);
152149

153-
// stylelint-disable-next-line selector-max-specificity, selector-max-combinators -- generated by polaris-migrator DO NOT COPY
154-
* + ul > &:first-of-type.focused::after {
155-
top: 1px;
150+
@media #{$p-breakpoints-sm-up} {
151+
border-radius: var(--p-border-radius-3);
152+
}
153+
}
154+
155+
// stylelint-disable-next-line selector-max-class -- set border radius for selectable items
156+
&.selectable {
157+
border-radius: var(--p-border-radius-0-experimental);
158+
159+
@media #{$p-breakpoints-sm-up} {
160+
// stylelint-disable-next-line -- set border radius for last selectable item to match ResourceList border radius
161+
&:last-child {
162+
border-bottom-left-radius: var(--p-border-radius-3);
163+
border-bottom-right-radius: var(--p-border-radius-3);
164+
}
165+
166+
// stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
167+
&.hasBulkActions {
168+
// stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
169+
&:last-child {
170+
// stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
171+
&.selected {
172+
border-bottom-left-radius: var(--p-border-radius-0-experimental);
173+
border-bottom-right-radius: var(--p-border-radius-0-experimental);
174+
}
175+
}
176+
}
177+
}
178+
}
156179
}
157180
}

polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,22 @@ export function SelectableWithMedia() {
7878
name: 'Yi So-Yeon',
7979
location: 'Gwangju, South Korea',
8080
},
81+
{
82+
id: '146',
83+
url: '#',
84+
avatarSource:
85+
'https://burst.shopifycdn.com/photos/woman-standing-in-front-of-yellow-background.jpg?width=746',
86+
name: 'Jane Smith',
87+
location: 'Manhattan, New York',
88+
},
89+
{
90+
id: '147',
91+
url: '#',
92+
avatarSource:
93+
'https://burst.shopifycdn.com/photos/relaxing-in-headphones.jpg?width=746',
94+
name: 'Grace Baker',
95+
location: 'Los Angeles, California',
96+
},
8197
]}
8298
renderItem={(item) => {
8399
const {id, url, avatarSource, name, location} = item;

polaris-react/src/components/ResourceItem/ResourceItem.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ class BaseResourceItem extends Component<CombinedProps, State> {
225225
styles.ListItem,
226226
focused && !focusedInner && styles.focused,
227227
hasBulkActions && styles.hasBulkActions,
228+
selected && styles.selected,
229+
selectable && styles.selectable,
228230
);
229231

230232
let actionsMarkup: React.ReactNode | null = null;

polaris-react/src/components/ResourceList/ResourceList.stories.tsx

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export default {
2020

2121
export function Default() {
2222
return (
23-
<Card padding="0">
23+
<Card padding="0" roundedAbove="sm">
2424
<ResourceList
2525
resourceName={{singular: 'customer', plural: 'customers'}}
2626
items={[
@@ -94,7 +94,7 @@ export function WithEmptyState() {
9494
<Page title="Files">
9595
<Layout>
9696
<Layout.Section>
97-
<Card padding="0">
97+
<Card padding="0" roundedAbove="sm">
9898
<ResourceList
9999
emptyState={emptyStateMarkup}
100100
items={items}
@@ -133,7 +133,7 @@ export function WithSelectionAndNoBulkActions() {
133133
];
134134

135135
return (
136-
<Card padding="0">
136+
<Card padding="0" roundedAbove="sm">
137137
<ResourceList
138138
resourceName={resourceName}
139139
items={items}
@@ -213,7 +213,7 @@ export function WithBulkActions() {
213213
];
214214

215215
return (
216-
<Card padding="0">
216+
<Card padding="0" roundedAbove="sm">
217217
<ResourceList
218218
resourceName={resourceName}
219219
items={items}
@@ -290,7 +290,7 @@ export function WithBulkActionsAndManyItems() {
290290
];
291291

292292
return (
293-
<Card padding="0">
293+
<Card padding="0" roundedAbove="sm">
294294
<ResourceList
295295
resourceName={resourceName}
296296
items={items}
@@ -371,7 +371,7 @@ export function WithLoadingState() {
371371
];
372372

373373
return (
374-
<Card padding="0">
374+
<Card padding="0" roundedAbove="sm">
375375
<ResourceList
376376
resourceName={resourceName}
377377
items={items}
@@ -409,7 +409,7 @@ export function WithLoadingState() {
409409

410410
export function WithTotalCount() {
411411
return (
412-
<Card padding="0">
412+
<Card padding="0" roundedAbove="sm">
413413
<ResourceList
414414
resourceName={{singular: 'customer', plural: 'customers'}}
415415
items={[
@@ -455,7 +455,7 @@ export function WithTotalCount() {
455455

456456
export function WithHeaderContent() {
457457
return (
458-
<Card padding="0">
458+
<Card padding="0" roundedAbove="sm">
459459
<ResourceList
460460
headerContent="Customer details shown below"
461461
items={[
@@ -522,7 +522,7 @@ export function WithSorting() {
522522
];
523523

524524
return (
525-
<Card padding="0">
525+
<Card padding="0" roundedAbove="sm">
526526
<ResourceList
527527
resourceName={resourceName}
528528
items={items}
@@ -584,7 +584,7 @@ export function WithAlternateTool() {
584584
];
585585

586586
return (
587-
<Card padding="0">
587+
<Card padding="0" roundedAbove="sm">
588588
<ResourceList
589589
items={items}
590590
renderItem={renderItem}
@@ -694,7 +694,7 @@ export function WithFiltering() {
694694
);
695695

696696
return (
697-
<Card padding="0">
697+
<Card padding="0" roundedAbove="sm">
698698
<ResourceList
699699
resourceName={resourceName}
700700
items={items}
@@ -806,7 +806,7 @@ export function WithACustomEmptySearchResultState() {
806806
);
807807

808808
return (
809-
<Card padding="0">
809+
<Card padding="0" roundedAbove="sm">
810810
<ResourceList
811811
resourceName={resourceName}
812812
items={items}
@@ -853,7 +853,7 @@ export function WithACustomEmptySearchResultState() {
853853

854854
export function WithItemShortcutActions() {
855855
return (
856-
<Card padding="0">
856+
<Card padding="0" roundedAbove="sm">
857857
<ResourceList
858858
resourceName={{singular: 'customer', plural: 'customers'}}
859859
items={[
@@ -909,7 +909,7 @@ export function WithItemShortcutActions() {
909909

910910
export function WithPersistentItemShortcutActions() {
911911
return (
912-
<Card padding="0">
912+
<Card padding="0" roundedAbove="sm">
913913
<ResourceList
914914
resourceName={{singular: 'customer', plural: 'customers'}}
915915
items={[
@@ -1034,7 +1034,7 @@ export function WithMultiselect() {
10341034
];
10351035

10361036
return (
1037-
<Card padding="0">
1037+
<Card padding="0" roundedAbove="sm">
10381038
<ResourceList
10391039
resourceName={resourceName}
10401040
items={items}
@@ -1183,7 +1183,7 @@ export function WithAllOfItsElements() {
11831183
);
11841184

11851185
return (
1186-
<Card padding="0">
1186+
<Card padding="0" roundedAbove="sm">
11871187
<ResourceList
11881188
resourceName={resourceName}
11891189
items={items}

0 commit comments

Comments
 (0)