Skip to content

Commit

Permalink
[Listbox, OptionList] Right align check icons on selected list items (#…
Browse files Browse the repository at this point in the history
…11697)

### WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-internal/issues/1503

### WHAT is this pull request doing?

Change CheckIcon placement from right to left alignment. 
Adds a placeholder space for non-selected options


[Figma](https://www.figma.com/file/I3df2veCGRVUuxzpXVE71m/Pickers?type=design&node-id=1655%3A23178&mode=design&t=ojAxTxPUmN0SoZYW-1)

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

  Include a video if your changes include interactive content.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

  <details>
    <summary>Summary of your gif(s)</summary>
    <img src="..." alt="Description of what the gif shows">
  </details>
-->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [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

- [ ] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] 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
  • Loading branch information
kyledurand committed Apr 3, 2024
1 parent c498099 commit 2adcbb5
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-timers-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Fixed layout shift for option lists within popovers
5 changes: 5 additions & 0 deletions .changeset/strong-otters-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Changed selected icon position in Listbox and OptionList
7 changes: 2 additions & 5 deletions polaris-react/src/components/Listbox/Listbox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,8 @@ export function WithAction() {
<Listbox.Option value="UniqueValue-2" divider>
Item 2
</Listbox.Option>
<Listbox.Action value="ActionValue">
<LegacyStack spacing="tight">
<Icon source={PlusCircleIcon} tone="base" />
<div>Add item</div>
</LegacyStack>
<Listbox.Action value="ActionValue" icon={PlusCircleIcon}>
Add item
</Listbox.Action>
</Listbox>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,3 @@
.ActionDivider {
margin-bottom: var(--p-space-100);
}

.Icon {
padding-right: var(--p-space-200);
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,21 @@ export const TextOption = memo(function TextOption({
isAction && styles.isAction,
);

const optionMarkup = selected ? (
const placeholder = isAction ? null : <Box width="20px" />;

const optionMarkup = (
<Box width="100%">
<InlineStack wrap={false} align="space-between" gap="200">
<InlineStack wrap={false} gap="100">
{selected ? (
<span>
<Icon source={CheckIcon} />
</span>
) : (
placeholder
)}
{children}
<InlineStack align="end">
<Icon source={CheckIcon} />
</InlineStack>
</InlineStack>
</Box>
) : (
<>{children}</>
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

display: flex;
flex-wrap: nowrap;
justify-content: space-between;
gap: var(--p-space-100);

/* stylelint-disable-next-line selector-max-specificity -- required for focus-visible support */
&.focused:focus-visible:not(:active) {
Expand Down Expand Up @@ -134,14 +134,6 @@
align-items: flex-end;
}

.Icon {
margin-left: var(--p-space-200);

svg {
fill: var(--p-color-icon-brand);
}
}

.Checkbox {
box-sizing: border-box;
display: flex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {classNames, variationName} from '../../../../utilities/css';
import type {InlineStackProps} from '../../../InlineStack';
import {InlineStack} from '../../../InlineStack';
import {Checkbox as PolarisCheckbox} from '../../../Checkbox';
import {Box} from '../../../Box';

import styles from './Option.module.css';

Expand Down Expand Up @@ -125,20 +126,27 @@ export function Option({
onBlur={toggleFocused}
aria-pressed={active || select}
>
<>
<InlineStack
wrap={false}
blockAlign={verticalAlignToBlockAlign(verticalAlign)}
{select || active ? (
<span className={styles.Icon}>
<Icon source={CheckIcon} tone="base" />
</span>
) : (
<Box width="20px" />
)}
<InlineStack
wrap={false}
blockAlign={verticalAlignToBlockAlign(verticalAlign)}
>
{mediaMarkup}
<span
style={{
minWidth:
typeof label === 'string' ? `${label.length}ch` : 'initial',
}}
>
{mediaMarkup}
{label}
</InlineStack>
{(select || active) && (
<span className={styles.Icon}>
<Icon source={CheckIcon} />
</span>
)}
</>
</span>
</InlineStack>
</button>
);

Expand Down

0 comments on commit 2adcbb5

Please sign in to comment.