[EmptyState] Add content context variant #1570

chloerice · 2019-05-24T21:32:00Z

WHY are these changes introduced?

This PR is one of two that will enable the improvement of transitions from skeleton loading states to empty states by adding support for use of the empty state component in a content context. For example, within a Card or ResourceList**.

WHAT is this pull request doing?

Adds content context consumer to EmptyState

Empty state in a card (small screen on right):

**Note: I'm adding empty state handling to ResourceList in a separate PR.

How to 🎩

Click to view collapsed tophatting instructions

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

View the "Empty state in content context" example in the Chroma deployment. The latest deployment can be found by clicking the "Details" link in the ci/chroma check line item of the PR checks below.

I recommend reviewing the code with whitespace changes removed from the diff so it's easier to see what actually changed (since I converted the component from a class to a function, most of the code in EmptyState.tsx shifted left one tab).

or

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

import React, {useState, useCallback} from 'react';
import {Page, Layout, EmptyState, Card} from '../src';

export function Playground() {
  const [contentContext, setContentContext] = useState(true);
  const toggleContentContext = useCallback(
    () => setContentContext((contentContext) => !contentContext),
    [],
  );

  const emptyStateMarkup = contentContext ? (
    <Card sectioned>
      <EmptyState
        heading="Upload a file to get started"
        action={{content: 'Upload files'}}
        image="https://user-images.githubusercontent.com/18447883/59945230-fa4be980-9434-11e9-9106-a373f0efbe08.png"
      >
        <p>
          You can use the Files section to upload images, videos, and other
          documents
        </p>
      </EmptyState>
    </Card>
  ) : (
    <EmptyState
      heading="Manage your inventory transfers"
      action={{content: 'Add transfer'}}
      secondaryAction={{content: 'Learn more', url: 'https://help.shopify.com'}}
      image="https://cdn.shopify.com/s/files/1/0757/9955/files/empty-state.svg"
    >
      <p>Track and receive your incoming inventory from suppliers.</p>
    </EmptyState>
  );

  const markup = contentContext ? (
    <Card>{emptyStateMarkup}</Card>
  ) : (
    emptyStateMarkup
  );

  const secondaryLayoutSection = contentContext ? (
    <Layout.Section secondary>{markup}</Layout.Section>
  ) : null;

  const title = contentContext
    ? 'Empty state content context'
    : 'Empty state standard';

  return (
    <Page
      title={title}
      primaryAction={{
        content: 'Toggle content context',
        onAction: toggleContentContext,
      }}
    >
      <Layout>
        <Layout.Section>{markup}</Layout.Section>
        {secondaryLayoutSection}
      </Layout>
    </Page>
  );
}

How to test your use case in a content context 👩‍🔬

Click to view collapsed instructions for testing your use case

Add a comment to this PR with an explanation of your use case with your empty state image attached/embedded in the comment.

For example, the image attached below is here so the playground code collapsed above will work for anyone locally. This is because the EmptyState image prop has the url of the attachment set as its string value.

You can copy the link to your image attachment by right clicking on the image and selecting "Copy image address" from the right click menu.

PR checklist

Tested on mobile
Tested on multiple browsers
~~(n/a) Tested for accessibility~~
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide

Ipkhchan · 2019-07-17T14:18:19Z

Uploading image to test with our use case on POS Channel Staff!

Ipkhchan · 2019-07-17T14:47:33Z

Our use case is within a ResourceList that displays POS staff members. If a shop does not have POS staff members, we want to show an empty state within the resource list that prompts them to add staff rather than just the default resource list empty state that just indicates that the shop has no staff.
Our workaround right now is to conditionally render the empty state vs. the resource list based on if there are staff. It would be nice to pass in an empty state to the resource list instead. Great that know someone's working on this!

Below is the design for the empty state provided by @pauyeung.

Below is the result of tossing our content (img and text) into the modified EmptyState implemented by this PR.

The EmptyState currently requires there to be a button in the content, whereas our design does not have a button within the empty state because our pageAction in the top right provides the call-to-action.

@pauyeung perhaps you can comment on this and how this looks?

pauyeung · 2019-07-17T15:22:11Z

@Ipkhchan @chloerice - I modelled my design off of this example in the Vault update. If buttons are required, does that mean there should be double parked primary action in both the page actions and resource list (Ex. Customers, Products)? I think it could be heavy handed

chloerice · 2019-07-22T18:30:58Z

Thanks for sharing your use cases @Ipkhchan @pauyeung!

The EmptyState currently requires there to be a button in the content, whereas our design does not have a button within the empty state because our pageAction in the top right provides the call-to-action.

I modelled my design off of this example in the Vault update. If buttons are required, does that mean there should be double parked primary action in both the page actions and resource list (Ex. Customers, Products)? I think it could be heavy handed

Having an action in the EmptyState is no longer required 🎉. Your page level action is enough in this use case to give merchants a next step 👍.

chloerice · 2019-07-22T18:32:42Z

Work on this functionality will be picked up again at the next Frideations.

dleroux · 2019-09-18T18:28:18Z

playground/images/index.ts

@@ -0,0 +1 @@
+export {default as FileUploadSpot} from './File-Upload-Spot.png';


Thanks for catching, meant to 🔥 this! That was left over from when I'd saved the empty state image locally before figuring out I could reference images attached to GitHub comments 👍.

dleroux · 2019-09-18T18:37:14Z

src/components/EmptyState/EmptyState.tsx

-        <TextContainer>{footerContent}</TextContainer>
-      </div>
-    ) : null;
+        const headingSize = withinContentContainer ? 'small' : 'medium';


Maybe moving the DisplayText Size type to src/types to use here, there and in tests would be better.

To clarify, you mean I should change DisplayText.size to use an enum instead of a union type so that I can use that there and here instead?

^ Just pushed up that change, but it will be a breaking change, at least for anyone consuming @shopify/polaris in a TypeScript project (web etc). I'm not sure it's worth that, @dleroux @BPScott let me know what you think.

I guess you are right, it's not worth the breaking change. Didn't realize it would be. We could just share the current type but not sure there's much value.

dleroux · 2019-09-18T18:41:15Z

src/components/EmptyState/EmptyState.scss

-  width: $detail-width;
+.withinContentContainer {
+  .Section {
+    position: unset;


do you need unset?

Yes, because the position is set to relative, when not in a content context, the position has to be unset or it still has a relative position which pushes it to the right a noticeable amount.

dleroux

Looks good except for the image that is accidentally being committed and left a comment on the size type.

BPScott · 2019-09-18T20:03:18Z

src/components/EmptyState/EmptyState.tsx

+  footerContent,
+}: EmptyStateProps) {
+  return (
+    <WithinContentContext.Consumer>


As you're in a functional component you can use hooks instead of the render prop function

const withinContentContainer = useContext(WithinContentContext); /* stuff using withinContentContainer */

instead of

return ( <WithinContentContext.Consumer> {(withinContentContainer) => { /* stuff using withinContentContainer */ }} </WithinContentContext.Consumer> );

Gotcha, thanks!

sarahill

This is looking good! On some mobile devices, the horizontal spacing feels tight. Can we add some left/right padding?

Before	After

chloerice · 2019-09-19T19:08:24Z

This is looking good! On some mobile devices, the horizontal spacing feels tight. Can we add some left/right padding?

Thanks @sarahill! I'm hesitant to add spacing around the EmptyState component directly, even just for the "in content context" variant. We generally handle the spacing of elements within the component, and leave it to the consumer to handle any spacing needed between and around components (by using structural Polaris components or custom styled elements as parent containers).

Nesting the EmptyState inside of a Card.Section takes care of the spacing in the case of a Card being the parent container. I'll update the example and playground code!

(Discussed and agreed that requested change would be breaking)

sarahill

✅

github-actions · 2019-10-01T23:40:58Z

Results

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified	5
Files potentially affected	1

Details

All files potentially affected (total: 1)

/src/components/EmptyState/EmptyState.tsx

❔ UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/EmptyState/EmptyState.scss (total: 1)

Files potentially affected (total: 1)

/src/components/EmptyState/EmptyState.tsx

🧩 src/components/EmptyState/EmptyState.tsx (total: 0)

Files potentially affected (total: 0)

❔ src/components/EmptyState/README.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/EmptyState/tests/EmptyState.test.tsx (total: 0)

Files potentially affected (total: 0)

This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

chloerice · 2019-10-01T23:43:22Z

src/components/EmptyState/tests/EmptyState.test.tsx

  let imgSrc =
    'https://cdn.shopify.com/s/files/1/0757/9955/files/empty-state.svg';

-  describe('primaryAction', () => {


(There is no primaryAction prop, it's just action)

BPScott deployed to polaris-react-pr-1570 May 24, 2019 21:32 View deployment

alex-page assigned chloerice Jul 31, 2019

chloerice mentioned this pull request Sep 1, 2019

ResourceList shows nothing when empty #1120

Closed

chloerice force-pushed the empty-state-content-context branch 2 times, most recently from 799cf7b to 9f7c1c4 Compare September 17, 2019 22:41

chloerice changed the title ~~[WIP][EmptyState] Add content context variant~~ [EmptyState] Add content context variant Sep 17, 2019

chloerice force-pushed the empty-state-content-context branch 4 times, most recently from cc7ed90 to 3b65fbc Compare September 18, 2019 14:57

chloerice requested review from dleroux, jessebc and sarahill September 18, 2019 14:58

chloerice mentioned this pull request Sep 18, 2019

[ResourceList] Add support for in context empty state #2160

Merged

6 tasks

dleroux reviewed Sep 18, 2019

View reviewed changes

dleroux previously requested changes Sep 18, 2019

View reviewed changes

BPScott reviewed Sep 18, 2019

View reviewed changes

chloerice force-pushed the empty-state-content-context branch 2 times, most recently from f966084 to b63293f Compare September 18, 2019 21:32

dleroux self-requested a review September 19, 2019 12:13

sarahill reviewed Sep 19, 2019

View reviewed changes

chloerice force-pushed the empty-state-content-context branch 2 times, most recently from 0a8e15f to 332db66 Compare September 19, 2019 21:07

sarahill approved these changes Sep 20, 2019

View reviewed changes

chloerice force-pushed the empty-state-content-context branch from 332db66 to 3030987 Compare October 1, 2019 23:38

chloerice commented Oct 1, 2019

View reviewed changes

chloerice force-pushed the empty-state-content-context branch 2 times, most recently from f8d7d7a to bda26c9 Compare October 1, 2019 23:47

chloerice added 5 commits November 26, 2019 14:15

[EmptyState] Add content context variant

1632ea6

[EmptyState] convert to functional component

187ec9a

[EmptyState] add tests for content context dependent conditionals

a518d79

[EmptyState] update change log

b31edb3

[EmptyState] add content context example

9b0ad21

chloerice force-pushed the empty-state-content-context branch from bda26c9 to 9b0ad21 Compare November 26, 2019 20:17

chloerice merged commit 9820e3a into master Nov 26, 2019

chloerice mentioned this pull request Nov 27, 2019

[Filters] Fix example spacing; [EmptyState] Fix prop descriptions and cascade order #2477

Merged

4 tasks

dleroux temporarily deployed to production December 4, 2019 14:42 Inactive

chloerice deleted the empty-state-content-context branch December 17, 2019 20:33

chloerice mentioned this pull request Jan 8, 2020

Refactor and revert revert resourcelist emptystate #2569

Merged

6 tasks

		@@ -0,0 +1 @@
		export {default as FileUploadSpot} from './File-Upload-Spot.png';

[EmptyState] Add content context variant #1570

[EmptyState] Add content context variant #1570

Uh oh!

Conversation

chloerice commented May 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

How to test your use case in a content context 👩‍🔬

PR checklist

Uh oh!

Ipkhchan commented Jul 17, 2019

Uh oh!

Ipkhchan commented Jul 17, 2019

Uh oh!

pauyeung commented Jul 17, 2019

Uh oh!

chloerice commented Jul 22, 2019

Uh oh!

chloerice commented Jul 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chloerice Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dleroux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarahill left a comment

Choose a reason for hiding this comment

Uh oh!

chloerice commented Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarahill left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Results

Details

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chloerice commented May 24, 2019 •

edited

Loading

chloerice Sep 18, 2019 •

edited

Loading

chloerice commented Sep 19, 2019 •

edited

Loading

github-actions bot commented Oct 1, 2019 •

edited

Loading