-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ResourceItem] Fix broken focus ring styles #10268
Conversation
5689bc5
to
af25e28
Compare
af25e28
to
b332eb7
Compare
@@ -0,0 +1,5 @@ | |||
--- | |||
'@shopify/polaris': patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this bug existed prior to se23, I've added a changeset to track the fix.
@@ -78,6 +78,22 @@ export function SelectableWithMedia() { | |||
name: 'Yi So-Yeon', | |||
location: 'Gwangju, South Korea', | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more items to test focus ring border styles for first/second/last children for ResourceItem
.
@@ -20,7 +20,7 @@ export default { | |||
|
|||
export function Default() { | |||
return ( | |||
<Card padding="0"> | |||
<Card padding="0" roundedAbove="sm"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added roundedAbove="sm"
prop to ResourceList stories to match styling on IndexTable.
@@ -120,38 +120,59 @@ | |||
@include item-separator; | |||
} | |||
|
|||
&:not(.hasBulkActions):not(.selectMode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored this logic since we're no longer relying on ::after
and moved hasBulkActions
focus ring styles to inside .focused
class.
} | ||
|
||
// stylelint-disable-next-line -- set border radius for last selectable item to match BulkActions border radius | ||
&.hasBulkActions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
712f08f
to
76d9c8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome refactor and clean up @laurkim 💯 looks great
### 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
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to next, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `next` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `next`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @shopify/polaris@12.0.0-beta.1 ### Major Changes - [#10122](#10122) [`43b42aefed`](43b42ae) Thanks [@aveline](https://github.com/aveline)! - Removed `shape` prop on `Avatar` component - [#10270](#10270) [`b9bcaef41`](b9bcaef) Thanks [@kyledurand](https://github.com/kyledurand)! - Changed `spacing` prop to `gap` on `List` and `DescriptionList` - [#9997](#9997) [`b59fc9e27`](b59fc9e) Thanks [@yurm04](https://github.com/yurm04)! - [ButtonGroup] Removed `segmented` boolean prop and replaced with `variant`. Replaced `spacing` prop with `gap` - [#10100](#10100) [`4c7b2d4858`](4c7b2d4) Thanks [@kyledurand](https://github.com/kyledurand)! - Updated `borderRadius` props to match web spec - [#10051](#10051) [`69edd97ceb`](69edd97) Thanks [@aveline](https://github.com/aveline)! - Renamed `color` prop to `tone` for `ProgressBar` component - [#10182](#10182) [`e814c0ee1a`](e814c0e) Thanks [@kyledurand](https://github.com/kyledurand)! - Removed connectedDislosure prop on button - [#10283](#10283) [`42ee9f407`](42ee9f4) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Renamed `size` prop values for the Avatar component. See the following table for the new prop mappings. | Before | After | | ------------------------- | ----------- | | `size="extraSmall"` | `size="xs"` | | `size="small"` | `size="sm"` | | `size="medium"` | `size="md"` | | `size="large"` | `size="lg"` | | `size="xl-experimental"` | `size="xl"` | | `size="2xl-experimental"` | `size="xl"` | - [#10232](#10232) [`eb2c2035c`](eb2c203) Thanks [@laurkim](https://github.com/laurkim)! - Removed `divider` prop from `Page` component - [#10271](#10271) [`1125087b5`](1125087) Thanks [@kyledurand](https://github.com/kyledurand)! - Removed deprecated additionalNavigation prop on Page Header - [#10164](#10164) [`af9f264b9a`](af9f264) Thanks [@aveline](https://github.com/aveline)! - Renamed `destructive` prop to `tone` for `Button` component - [#10261](#10261) [`abeef7ad0`](abeef7a) Thanks [@kyledurand](https://github.com/kyledurand)! - Replaced `small`, `large`, and `fullScreen` props with `size` prop - [#10206](#10206) [`dad09bde9`](dad09bd) Thanks [@kyledurand](https://github.com/kyledurand)! - Changed `status` prop on `Banner` to `tone` - [#10220](#10220) [`2b0cdb2fbf`](2b0cdb2) Thanks [@kyledurand](https://github.com/kyledurand)! - Changed `color` prop on `Icon` to `tone` - [#10036](#10036) [`359614cf83`](359614c) Thanks [@kyledurand](https://github.com/kyledurand)! - Replaced `borderless` prop with `variant` on TextField Migration: `npx @shopify/polaris-migrator react-rename-component-prop <path> --componentName="TextField" --from="borderless" --to="variant" --newValue="borderless"` - [#10635](#10635) [`340e36e7d`](340e36e) Thanks [@laurkim](https://github.com/laurkim)! - Removed `polarisSummerEditions2023` feature flag from AppProvider context - [#10090](#10090) [`4caed28a77`](4caed28) Thanks [@aveline](https://github.com/aveline)! - Consolidated boolean `Button` props into `variant` prop - [#10041](#10041) [`8f927f7622`](8f927f7) Thanks [@kyledurand](https://github.com/kyledurand)! - Replaced boolean props: `secondary`, `fullWidth`, `oneHalf`, `oneThird` on Layout.Section with `variant` - [#10266](#10266) [`22a51eae2`](22a51ea) Thanks [@kyledurand](https://github.com/kyledurand)! - Renamed `color` prop on Text to `tone` - [#9993](#9993) [`66f5c8df3e`](66f5c8d) Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Removed Summer Editions experimental styles and code for the following components: `AppProvider`, `Avatar`, `AccountConnection`, `ActionList`, `ActionMenu`, `Autocomplete`, `Badge`, `Banner`, `Breadcrumbs`, `BulkActions`, `Button`, `ButtonGroup`, `CalloutCard`, `Card`, `CheckableButton`, `Checkbox`, `Choice`, `Connected`, `DataTable`, `DatePicker`, `DropZone`, `EmptyState`, `Filters`, `FormLayout`, `Frame`, `FullscreenBar`, `IndexFilters`, `IndexTable`, `InlineError`, `KeyboardKey`, `Labelled`, `Layout`, `LegacyCard`, `LegacyFilters`, `LegacyTabs`, `Link`, `List`, `Listbox`, `MediaCard`, `Modal`, `Navigation`, `OptionList`, `Page`, `PageActions`, `Pagination`, `Popover`, `ProgressBar`, `RadioButton`, `ResourceItem`, `ResourceList`, `Select`, `SettingAction`, `ShadowBevel`, `SkeletonPage`, `SkeletonThumbnail`, `Spinner`, `Tabs`, `Tag`, `Text`, `TextField`, `Thumbnail`, `TooltipOverlay`, `TopBar`, and `VideoThumbnail` - [#10232](#10232) [`eb2c2035c`](eb2c203) Thanks [@laurkim](https://github.com/laurkim)! - Removed `optionRole` prop from `OptionList` component ### Minor Changes - [#10238](#10238) [`b17d23d69`](b17d23d) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Add a search field to filter ActionList that have more than 10 items ### Patch Changes - [#10228](#10228) [`e18ca907e`](e18ca90) Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - `Filters` Remove unused disabled states in `FilterPill` - [#10268](#10268) [`dbe3d9ece`](dbe3d9e) Thanks [@laurkim](https://github.com/laurkim)! - Fixed broken focus ring styles on `ResourceItem` component - [#10238](#10238) [`b17d23d69`](b17d23d) Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Ensure Avatar has no background color if an source prop is passed in to allow for transparent images - [#10230](#10230) [`a573e55d0`](a573e55) Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - `IndexFilter` remove custom `FilterButton` in favor of directly invoking the Polaris `Button` component. - Updated dependencies \[[`86d4040c0`](86d4040)]: - @shopify/polaris-tokens@7.13.0-beta.0 ## @shopify/polaris-tokens@7.13.0-beta.0 ### Minor Changes - [#10382](#10382) [`86d4040c0`](86d4040) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `ThemeVariant` to `Theme` and exposed `Theme` type ## @shopify/stylelint-polaris@14.1.2-beta.0 ### Patch Changes - Updated dependencies \[[`86d4040c0`](86d4040)]: - @shopify/polaris-tokens@7.13.0-beta.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
### WHY are these changes introduced? Resolves Shopify#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
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 thefocus-ring
mixin.ResourceItem focus state — before
ResourceItem focus state — after
How to 🎩
ResourceItem Storybook
ResourceItem Prod Storybook
BulkActions Storybook
BulkActions Prod Storybook
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.md
with documentation changes