[PageActions] Add ReactNode as an accepted secondaryActions prop value for customizability #5495

stefanlegg · 2022-04-11T19:30:03Z

WHY are these changes introduced?

This PR is related to a similar recent change to secondaryActions on Page.

Currently secondaryActions on PageActions only allows for passing props as ComplexAction[] which doesn't allow for flexibility and results in accessibility issues in some cases.

In cases where secondary actions open a modal we are unable to move focus back to the button that opened it after the modal is closed. With this change to accept React.ReactNode we are better able to support keyboard users through passing an entire modal as secondary action to leverage modal's built-in focus management, or through passing a custom button that'd able to have a ref to receive the focus.

WHAT is this pull request doing?

This PR maintains the existing secondaryActions prop, while adding the option to pass a ReactNode instead for customizability.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React, {useCallback, useState} from 'react';

import {Button, Modal, Page, PageActions, TextContainer} from '../src';

export function Playground() {
  return (
    <Page>
      <PageActions 
        primaryAction={{
          content: 'Save',
        }} 
        secondaryActions={<ModalExample />}
      />
    </Page>
  );
}

function ModalExample() {
  const [active, setActive] = useState(true);

  const handleChange = useCallback(() => setActive(!active), [active]);

  const activator = <Button onClick={handleChange}>Open</Button>;

  return (
    <Modal
      activator={activator}
      open={active}
      onClose={handleChange}
      title="Reach more shoppers with Instagram product tags"
      primaryAction={{
        content: 'Add Instagram',
        onAction: handleChange,
      }}
      secondaryActions={[
        {
          content: 'Learn more',
          onAction: handleChange,
        },
      ]}
    >
      <Modal.Section>
        <TextContainer>
          <p>
            Use Instagram posts to share your products with millions of people.
            Let shoppers buy from your store without leaving Instagram.
          </p>
        </TextContainer>
      </Modal.Section>
    </Modal>
  );
}

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide
For visual design changes, ping @ sarahill to update the Polaris UI kit

github-actions · 2022-04-11T19:34:01Z

size-limit report 📦

Path	Size
cjs	158.51 KB (+0.02% 🔺)
esm	89.73 KB (+0.02% 🔺)
esnext	139.1 KB (+0.01% 🔺)
css	36.63 KB (0%)

stefanlegg · 2022-04-11T19:57:18Z

polaris-react/src/components/PageActions/utilities.ts

+import {isValidElement} from 'react';
+
+export function isInterface<T>(x: T | React.ReactNode): x is T {
+  return !isValidElement(x) && x !== undefined;
+}
+
+export function isReactElement<T>(x: T): x is T {
+  return isValidElement(x) && x !== undefined;
+}


[question] These are the same util functions that are present for the Page component. Is there an appropriate place we could refactor both copies into? Maybe utilities/components.tsx?

The utilities/components.tsx file looks like a good place to me for such utilities if used globally for reusability, could polaris team confirm if our assumption is correct? 🙏

Yes that's a good idea, you can make individual files named similarly to is-object.ts 👍🏽

tatianau

Thanks for doing this, changes look good to me! 🚀
Let's wait for polaris team members to ✅ 🙏

tatianau · 2022-04-11T20:25:04Z

polaris-react/src/components/PageActions/utilities.ts

+import {isValidElement} from 'react';
+
+export function isInterface<T>(x: T | React.ReactNode): x is T {
+  return !isValidElement(x) && x !== undefined;
+}
+
+export function isReactElement<T>(x: T): x is T {
+  return isValidElement(x) && x !== undefined;
+}


The utilities/components.tsx file looks like a good place to me for such utilities if used globally for reusability, could polaris team confirm if our assumption is correct? 🙏

zaquille-oneil

LGTM!

chloerice · 2022-04-11T21:18:39Z

polaris-react/src/components/PageActions/README.md

+    <Button
+      connectedDisclosure={{
+        accessibilityLabel: 'Other save actions',
+        actions: [{content: 'Save as new'}],


Suggested change

actions: [{content: 'Save as new'}],

actions: [{content: 'Save as draft'}],

chloerice · 2022-04-11T21:19:47Z

polaris-react/UNRELEASED.md

@@ -8,6 +8,8 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t

 - Added `icon` prop to the `Badge` component ([#5292](https://github.com/Shopify/polaris/pull/5292))

+- Added `ReactNode` as an accepted prop type to `secondaryActions` on the `PageActions` component ([#5495](https://github.com/Shopify/polaris/pull/5495))


Suggested change

- Added `ReactNode` as an accepted prop type to `secondaryActions` on the `PageActions` component ([#5495](https://github.com/Shopify/polaris/pull/5495))

- Added support for setting a `ReactNode` on the `PageActions` `secondaryActions` prop ([#5495](https://github.com/Shopify/polaris/pull/5495))

chloerice · 2022-04-11T21:22:56Z

polaris-react/src/components/PageActions/utilities.ts

+import {isValidElement} from 'react';
+
+export function isInterface<T>(x: T | React.ReactNode): x is T {
+  return !isValidElement(x) && x !== undefined;
+}
+
+export function isReactElement<T>(x: T): x is T {
+  return isValidElement(x) && x !== undefined;
+}


Yes that's a good idea, you can make individual files named similarly to is-object.ts 👍🏽

chloerice · 2022-04-11T21:24:32Z

polaris-react/src/components/PageActions/README.md

+        actions: [{content: 'Save as new'}],
+      }}
+    >
+      Save as new


Suggested change

Save as new

Save

ghost · 2022-04-12T17:46:40Z

🎉 Thanks for your contribution to Polaris React!

stefanlegg force-pushed the sl-custom-secondary-actions branch from df9ca8d to 4be8a25 Compare April 11, 2022 19:34

stefanlegg self-assigned this Apr 11, 2022

stefanlegg commented Apr 11, 2022

View reviewed changes

stefanlegg requested review from alex-page, tatianau, ChrisMLee and zaquille-oneil April 11, 2022 19:58

stefanlegg marked this pull request as ready for review April 11, 2022 19:58

stefanlegg force-pushed the sl-custom-secondary-actions branch from 4be8a25 to a7aa2d7 Compare April 11, 2022 20:17

tatianau approved these changes Apr 11, 2022

View reviewed changes

zaquille-oneil reviewed Apr 11, 2022

View reviewed changes

chloerice approved these changes Apr 11, 2022

View reviewed changes

chloerice reviewed Apr 11, 2022

View reviewed changes

stefanlegg force-pushed the sl-custom-secondary-actions branch from a7aa2d7 to 3a6f374 Compare April 12, 2022 14:36

Allow for React.ReactNode in PageActions secondaryActions

d5888b6

stefanlegg force-pushed the sl-custom-secondary-actions branch from 3a6f374 to d5888b6 Compare April 12, 2022 14:51

stefanlegg merged commit bf787c9 into main Apr 12, 2022

stefanlegg deleted the sl-custom-secondary-actions branch April 12, 2022 17:46

rmleg mentioned this pull request May 25, 2022

[PageActions] Add ReactNode as an accepted primaryAction prop value for customizability #5946

Merged

5 tasks

tatianau mentioned this pull request Jan 19, 2023

[a11y][Page] Secondary actions should have an ability to receive focus #8105

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PageActions] Add ReactNode as an accepted secondaryActions prop value for customizability #5495

[PageActions] Add ReactNode as an accepted secondaryActions prop value for customizability #5495

stefanlegg commented Apr 11, 2022 •

edited

Loading

github-actions bot commented Apr 11, 2022 •

edited

Loading

stefanlegg Apr 11, 2022

tatianau Apr 11, 2022

chloerice Apr 11, 2022

tatianau left a comment

tatianau Apr 11, 2022

zaquille-oneil left a comment

chloerice Apr 11, 2022

chloerice Apr 11, 2022

chloerice Apr 11, 2022

chloerice Apr 11, 2022

ghost commented Apr 12, 2022

	actions: [{content: 'Save as new'}],
	actions: [{content: 'Save as draft'}],

		@@ -8,6 +8,8 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t

		- Added `icon` prop to the `Badge` component ([#5292](https://github.com/Shopify/polaris/pull/5292))

		- Added `ReactNode` as an accepted prop type to `secondaryActions` on the `PageActions` component ([#5495](https://github.com/Shopify/polaris/pull/5495))

	- Added `ReactNode` as an accepted prop type to `secondaryActions` on the `PageActions` component ([#5495](https://github.com/Shopify/polaris/pull/5495))
	- Added support for setting a `ReactNode` on the `PageActions` `secondaryActions` prop ([#5495](https://github.com/Shopify/polaris/pull/5495))

[PageActions] Add ReactNode as an accepted secondaryActions prop value for customizability #5495

[PageActions] Add ReactNode as an accepted secondaryActions prop value for customizability #5495

Conversation

stefanlegg commented Apr 11, 2022 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

🎩 checklist

github-actions bot commented Apr 11, 2022 • edited Loading

size-limit report 📦

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatianau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zaquille-oneil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Apr 12, 2022

stefanlegg commented Apr 11, 2022 •

edited

Loading

github-actions bot commented Apr 11, 2022 •

edited

Loading