[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
Shopify · Aug 30, 2023 · dbe3d9e · dbe3d9e
1 parent 1125087
commit dbe3d9e
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 43 deletions.
diff --git a/.changeset/light-seahorses-sit.md b/.changeset/light-seahorses-sit.md
@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+Fixed broken focus ring styles on `ResourceItem` component
diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.scss b/polaris-react/src/components/ResourceItem/ResourceItem.scss
@@ -111,7 +111,7 @@
 }
 
 .ListItem {
-  // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
+  // stylelint-disable-next-line -- include focus-ring mixin to prevent jump in content for first focused element
   @include focus-ring($border-width: -1px);
 
   .ListItem + & {
@@ -120,38 +120,61 @@
     @include item-separator;
   }
 
-  &:not(.hasBulkActions):not(.selectMode) {
-    // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
-    &::after {
-      border-radius: var(--p-border-radius-05);
-    }
+  &.focused {
+    // stylelint-disable-next-line -- remove focus-ring mixin to use outline styles
+    @include no-focus-ring;
+    outline: var(--p-border-width-2) solid
+      var(--p-color-border-interactive-focus);
+    outline-offset: calc(-1 * var(--p-space-05));
+    // stylelint-disable-next-line -- custom property
+    z-index: var(--pc-resource-item-clickable-stacking-order);
+    border-radius: var(--p-border-radius-0-experimental);
 
-    // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
-    &:last-of-type {
-      border-bottom-left-radius: var(--p-border-radius-2);
-      border-bottom-right-radius: var(--p-border-radius-2);
-      // stylelint-disable selector-max-specificity -- Needed due to the nesting of the elements that change border-radius based on selection mode
-      .ItemWrapper {
-        border-radius: inherit;
+    @media #{$p-breakpoints-sm-up} {
+      border-radius: var(--p-border-radius-3);
+
+      &:first-of-type {
+        border-bottom-left-radius: var(--p-border-radius-0-experimental);
+        border-bottom-right-radius: var(--p-border-radius-0-experimental);
       }
 
-      &.focused::after {
-        border-bottom-left-radius: var(--p-border-radius-2);
-        border-bottom-right-radius: var(--p-border-radius-2);
+      &:last-of-type {
+        border-top-left-radius: var(--p-border-radius-0-experimental);
+        border-top-right-radius: var(--p-border-radius-0-experimental);
       }
-      // stylelint-enable
     }
-  }
 
-  &.focused {
-    // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
-    @include focus-ring($style: 'focused');
-    // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY
-    z-index: var(--pc-resource-item-clickable-stacking-order);
-  }
+    &:only-child {
+      border-radius: var(--p-border-radius-0-experimental);
 
-  // stylelint-disable-next-line selector-max-specificity, selector-max-combinators -- generated by polaris-migrator DO NOT COPY
-  * + ul > &:first-of-type.focused::after {
-    top: 1px;
+      @media #{$p-breakpoints-sm-up} {
+        border-radius: var(--p-border-radius-3);
+      }
+    }
+
+    // stylelint-disable-next-line selector-max-class -- set border radius for selectable items
+    &.selectable {
+      border-radius: var(--p-border-radius-0-experimental);
+
+      @media #{$p-breakpoints-sm-up} {
+        // stylelint-disable-next-line -- set border radius for last selectable item to match ResourceList border radius
+        &:last-child {
+          border-bottom-left-radius: var(--p-border-radius-3);
+          border-bottom-right-radius: var(--p-border-radius-3);
+        }
+
+        // stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
+        &.hasBulkActions {
+          // stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
+          &:last-child {
+            // stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius
+            &.selected {
+              border-bottom-left-radius: var(--p-border-radius-0-experimental);
+              border-bottom-right-radius: var(--p-border-radius-0-experimental);
+            }
+          }
+        }
+      }
+    }
   }
 }
diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx b/polaris-react/src/components/ResourceItem/ResourceItem.stories.tsx
@@ -78,6 +78,22 @@ export function SelectableWithMedia() {
             name: 'Yi So-Yeon',
             location: 'Gwangju, South Korea',
           },
+          {
+            id: '146',
+            url: '#',
+            avatarSource:
+              'https://burst.shopifycdn.com/photos/woman-standing-in-front-of-yellow-background.jpg?width=746',
+            name: 'Jane Smith',
+            location: 'Manhattan, New York',
+          },
+          {
+            id: '147',
+            url: '#',
+            avatarSource:
+              'https://burst.shopifycdn.com/photos/relaxing-in-headphones.jpg?width=746',
+            name: 'Grace Baker',
+            location: 'Los Angeles, California',
+          },
         ]}
         renderItem={(item) => {
           const {id, url, avatarSource, name, location} = item;

diff --git a/polaris-react/src/components/ResourceItem/ResourceItem.tsx b/polaris-react/src/components/ResourceItem/ResourceItem.tsx
@@ -225,6 +225,8 @@ class BaseResourceItem extends Component<CombinedProps, State> {
       styles.ListItem,
       focused && !focusedInner && styles.focused,
       hasBulkActions && styles.hasBulkActions,
+      selected && styles.selected,
+      selectable && styles.selectable,
     );
 
     let actionsMarkup: React.ReactNode | null = null;

diff --git a/polaris-react/src/components/ResourceList/ResourceList.stories.tsx b/polaris-react/src/components/ResourceList/ResourceList.stories.tsx
@@ -20,7 +20,7 @@ export default {
 
 export function Default() {
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={{singular: 'customer', plural: 'customers'}}
         items={[
@@ -94,7 +94,7 @@ export function WithEmptyState() {
     <Page title="Files">
       <Layout>
         <Layout.Section>
-          <Card padding="0">
+          <Card padding="0" roundedAbove="sm">
             <ResourceList
               emptyState={emptyStateMarkup}
               items={items}
@@ -133,7 +133,7 @@ export function WithSelectionAndNoBulkActions() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -213,7 +213,7 @@ export function WithBulkActions() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -290,7 +290,7 @@ export function WithBulkActionsAndManyItems() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -371,7 +371,7 @@ export function WithLoadingState() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -409,7 +409,7 @@ export function WithLoadingState() {
 
 export function WithTotalCount() {
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={{singular: 'customer', plural: 'customers'}}
         items={[
@@ -455,7 +455,7 @@ export function WithTotalCount() {
 
 export function WithHeaderContent() {
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         headerContent="Customer details shown below"
         items={[
@@ -522,7 +522,7 @@ export function WithSorting() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -584,7 +584,7 @@ export function WithAlternateTool() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         items={items}
         renderItem={renderItem}
@@ -694,7 +694,7 @@ export function WithFiltering() {
   );
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -806,7 +806,7 @@ export function WithACustomEmptySearchResultState() {
   );
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -853,7 +853,7 @@ export function WithACustomEmptySearchResultState() {
 
 export function WithItemShortcutActions() {
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={{singular: 'customer', plural: 'customers'}}
         items={[
@@ -909,7 +909,7 @@ export function WithItemShortcutActions() {
 
 export function WithPersistentItemShortcutActions() {
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={{singular: 'customer', plural: 'customers'}}
         items={[
@@ -1034,7 +1034,7 @@ export function WithMultiselect() {
   ];
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}
@@ -1183,7 +1183,7 @@ export function WithAllOfItsElements() {
   );
 
   return (
-    <Card padding="0">
+    <Card padding="0" roundedAbove="sm">
       <ResourceList
         resourceName={resourceName}
         items={items}