-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ResourceList] Add support for in context empty state #2160
Conversation
8d1392a
to
491915e
Compare
0a8e15f
to
332db66
Compare
f8d7d7a
to
bda26c9
Compare
e6af687
to
25c002e
Compare
💦 Potential splash zone of changes introduced to No significant changes to This comment automatically updates as changes are made to this pull request. |
4cd32b8
to
47d7a26
Compare
@chloerice I think disabling the controls totally makes sense in this context! The only thing I'm wondering about is a |
Definitely 💯 Thanks for catching that!! |
47d7a26
to
a800bd2
Compare
This looks good to me! Same feedback as Devon on the tab-index. Other than that this looks good 👍 |
Agreed that this would be confusing. I think we should either disable them, or remove them completely. Let's start with keeping them disabled and see how that feels when we tophat it. |
Still need to figure out where that |
Quick update --- finally figured out that the |
e61ca8c
to
800923b
Compare
800923b
to
647e551
Compare
647e551
to
00a265c
Compare
Fixing in #2473! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@@ -471,9 +475,14 @@ class ResourceList extends React.Component<CombinedProps, State> { | |||
<div className={styles['HeaderWrapper-overlay']} /> | |||
) : null; | |||
|
|||
const showEmptyState = filterControl && !this.itemsExist() && !loading; | |||
const showNoResults = | |||
filterControl && !this.itemsExist() && hasMoreItems && !loading; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is what might be the issue. hasMoreItems
is used to create a link in the bulkActions
when there are more results than can be displayed. !this.itemsExist() && hasMoreItems
should not ever be true if I'm understanding this correctly.
…ptystate"" This reverts commit 6ec053f.
Revert "Merge pull request #2160 from Shopify/resourcelist-emptystate"
…ptystate"" This reverts commit 6ec053f.
…ptystate"" This reverts commit 6ec053f.
…ptystate"" This reverts commit 6ec053f.
…ptystate"" This reverts commit 6ec053f.
…ptystate"" This reverts commit 6ec053f.
WHY are these changes introduced?
Builds on #1570
Right now our empty states are vastly different from our loading and with-data states. We'd like to make the transition from loading to empty state smoother, as well as make the with-data state more familiar once a feature has been used. For list views, this means having our empty states within the context of the resource list.
WHAT is this pull request doing?
Currently,
ResourceList
only has an empty state for when there are no results for a filter or search query. This PR adds support for disabling filters and providing markup to render when there are not yet any resources to list. That way the same static content can be rendered in a loading, empty, and with-data state. See the tophatting instructions to view the smooth visual transition between these states using the playground code.** This is a Foundations Frideations project.
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
View the "Resource list with empty state" example
or
git checkout resourcelist-emptystate
playground/Playground.tsx
yarn dev
to run and open Storybook locallyCopy-paste this code in
playground/Playground.tsx
:🎩 checklist
README.md
with documentation changes