Revert "[Listbox, OptionList] Right align check icons on selected lis…

…t items" (Shopify#11838) Reverts Shopify#11697 Need to better accommodate custom elements within list items
AnnaCheba · Apr 4, 2024 · f7f3718 · f7f3718
1 parent d5cb5ea
commit f7f3718
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 44 deletions.
diff --git a/.changeset/chatty-timers-marry.md b/.changeset/chatty-timers-marry.md
diff --git a/.changeset/strong-otters-fetch.md b/.changeset/strong-otters-fetch.md
diff --git a/polaris-react/src/components/Listbox/Listbox.stories.tsx b/polaris-react/src/components/Listbox/Listbox.stories.tsx
@@ -102,8 +102,11 @@ export function WithAction() {
       <Listbox.Option value="UniqueValue-2" divider>
         Item 2
       </Listbox.Option>
-      <Listbox.Action value="ActionValue" icon={PlusCircleIcon}>
-        Add item
+      <Listbox.Action value="ActionValue">
+        <LegacyStack spacing="tight">
+          <Icon source={PlusCircleIcon} tone="base" />
+          <div>Add item</div>
+        </LegacyStack>
       </Listbox.Action>
     </Listbox>
   );

diff --git a/polaris-react/src/components/Listbox/components/Action/Action.module.css b/polaris-react/src/components/Listbox/components/Action/Action.module.css
@@ -6,3 +6,7 @@
 .ActionDivider {
   margin-bottom: var(--p-space-100);
 }
+
+.Icon {
+  padding-right: var(--p-space-200);
+}
diff --git a/polaris-react/src/components/Listbox/components/TextOption/TextOption.tsx b/polaris-react/src/components/Listbox/components/TextOption/TextOption.tsx
@@ -35,21 +35,17 @@ export const TextOption = memo(function TextOption({
     isAction && styles.isAction,
   );
 
-  const placeholder = isAction ? null : <Box width="20px" />;
-
-  const optionMarkup = (
+  const optionMarkup = selected ? (
     <Box width="100%">
-      <InlineStack wrap={false} gap="100">
-        {selected ? (
-          <span>
-            <Icon source={CheckIcon} />
-          </span>
-        ) : (
-          placeholder
-        )}
+      <InlineStack wrap={false} align="space-between" gap="200">
         {children}
+        <InlineStack align="end">
+          <Icon source={CheckIcon} />
+        </InlineStack>
       </InlineStack>
     </Box>
+  ) : (
+    <>{children}</>
   );
 
   return (

diff --git a/polaris-react/src/components/OptionList/components/Option/Option.module.css b/polaris-react/src/components/OptionList/components/Option/Option.module.css
@@ -26,7 +26,7 @@
 
   display: flex;
   flex-wrap: nowrap;
-  gap: var(--p-space-100);
+  justify-content: space-between;
 
   /* stylelint-disable-next-line selector-max-specificity -- required for focus-visible support */
   &.focused:focus-visible:not(:active) {
@@ -134,6 +134,14 @@
   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;

diff --git a/polaris-react/src/components/OptionList/components/Option/Option.tsx b/polaris-react/src/components/OptionList/components/Option/Option.tsx
@@ -11,7 +11,6 @@ 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';
 
@@ -126,27 +125,20 @@ export function Option({
       onBlur={toggleFocused}
       aria-pressed={active || select}
     >
-      {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',
-          }}
+      <>
+        <InlineStack
+          wrap={false}
+          blockAlign={verticalAlignToBlockAlign(verticalAlign)}
         >
+          {mediaMarkup}
           {label}
-        </span>
-      </InlineStack>
+        </InlineStack>
+        {(select || active) && (
+          <span className={styles.Icon}>
+            <Icon source={CheckIcon} />
+          </span>
+        )}
+      </>
     </button>
   );