[ActionList] Make spacing consistent

UNRELEASED.md

@@ -8,6 +8,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 - Added `id` prop to `Layout` and `Heading` for hash linking ([#4307](https://github.com/Shopify/polaris-react/pull/4307))
 - Added `external` prop to `Navigation.Item` component ([#4310](https://github.com/Shopify/polaris-react/pull/4310))
+- Added consistent spacing to `ActionList` ([#4355](https://github.com/Shopify/polaris-react/pull/4355))
 - Added `ariaLabelledBy` props to `Navigation` component to allow a hidden label for accessibility ([#4343](https://github.com/Shopify/polaris-react/pull/4343))
 - Add `lastColumnSticky` prop to `IndexTable` to create a sticky last cell and optional sticky last heading on viewports larger than small ([#4150](https://github.com/Shopify/polaris-react/pull/4150))
 

src/components/ActionList/ActionList.scss

@@ -3,6 +3,7 @@
 $image-size: rem(20px);
 $item-min-height: rem(40px);
 $item-vertical-padding: ($item-min-height - line-height(body)) / 2;
+$titleVerticalSpacing: spacing(tight) * 1.5;
 
 .ActionList {
   list-style: none;
@@ -12,20 +13,13 @@ $item-vertical-padding: ($item-min-height - line-height(body)) / 2;
 
 .Section-withoutTitle:not(:first-child) {
   border-top: border('divider');
-  margin-top: spacing(tight);
-  padding-top: spacing(tight);
 }
 
 .Actions {
   list-style: none;
   margin: 0;
   border-top: border('divider');
-  padding: 0 spacing(tight);
-
-  // stylelint-disable-next-line selector-max-class
-  .Title + & {
-    padding-top: spacing(tight);
-  }
+  padding: spacing(tight);
 }
 
 .ActionList,
@@ -38,7 +32,11 @@ $item-vertical-padding: ($item-min-height - line-height(body)) / 2;
 
 .Title {
   @include text-style-subheading;
-  padding: (spacing(tight) * 1.5) spacing();
+  padding: spacing(extra-tight) spacing() $titleVerticalSpacing spacing();
+
+  &.firstSectionWithTitle {
+    padding-top: $titleVerticalSpacing;
+  }
 }
 
 .Item {

src/components/ActionList/ActionList.tsx

@@ -39,6 +39,7 @@ export function ActionList({
     return section.items.length > 0 ? (
       <Section
         key={section.title || index}
+        firstSection={index === 0}
         section={section}
         hasMultipleSections={hasMultipleSections}
         actionRole={actionRole}

src/components/ActionList/components/Section/Section.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 
+import {classNames} from '../../../../utilities/css';
 import {Item} from '../Item';
 import type {
   ActionListItemDescriptor,
@@ -14,6 +15,8 @@ export interface SectionProps {
   hasMultipleSections: boolean;
   /** Defines a specific role attribute for each action in the list */
   actionRole?: string;
+  /** Whether or not the section is the first to appear */
+  firstSection?: boolean;
   /** Callback when any item is clicked or keypressed */
   onActionAnyItem?: ActionListItemDescriptor['onAction'];
 }
@@ -22,6 +25,7 @@ export function Section({
   section,
   hasMultipleSections,
   actionRole,
+  firstSection,
   onActionAnyItem,
 }: SectionProps) {
   const handleAction = (itemOnAction: ActionListItemDescriptor['onAction']) => {
@@ -50,9 +54,13 @@ export function Section({
   );
 
   const className = section.title ? undefined : styles['Section-withoutTitle'];
+  const titleClassName = classNames(
+    styles.Title,
+    firstSection && styles.firstSectionWithTitle,
+  );
 
   const titleMarkup = section.title ? (
-    <p className={styles.Title}>{section.title}</p>
+    <p className={titleClassName}>{section.title}</p>
   ) : null;
 
   const sectionRole = actionRole === 'option' ? 'presentation' : undefined;

src/components/ActionList/tests/ActionList.test.tsx

@@ -85,6 +85,41 @@ describe('<ActionList />', () => {
     });
   });
 
+  it('passes firstSection=true to the first Section', () => {
+    const actionList = mountWithApp(
+      <ActionList
+        sections={[
+          {title: 'One', items: [{content: 'First section'}]},
+          {title: 'Two', items: [{content: 'Second section'}]},
+        ]}
+        onActionAnyItem={mockOnActionAnyItem}
+        actionRole="option"
+      />,
+    );
+
+    const {firstSection} = actionList.find(Section)!.props;
+
+    expect(firstSection).toBe(true);
+  });
+
+  it('passes firstSection=false to sections that are not the first', () => {
+    const actionList = mountWithApp(
+      <ActionList
+        sections={[
+          {title: 'One', items: [{content: 'First section'}]},
+          {title: 'Two', items: [{content: 'Second section'}]},
+        ]}
+        onActionAnyItem={mockOnActionAnyItem}
+        actionRole="option"
+      />,
+    );
+
+    const sections = actionList.findAll(Section);
+    const {firstSection} = sections[sections.length - 1].props;
+
+    expect(firstSection).toBe(false);
+  });
+
   it('renders a ul with sections', () => {
     const actionList = mountWithApp(
       <ActionList