fix(PaginationTableDemo): add Spinner and empty state to demo by nicolethoen · Pull Request #3294 · patternfly/patternfly-react · GitHub

Codecov Report

❗ No coverage uploaded for pull request base (master@2415a94). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3294   +/-   ##
=========================================
  Coverage          ?   67.44%           
=========================================
  Files             ?      892           
  Lines             ?    24874           
  Branches          ?     2141           
=========================================
  Hits              ?    16776           
  Misses            ?     7093           
  Partials          ?     1005

Flag	Coverage Δ
#misc	95.45% <ø> (?)
#patternfly3	69.29% <ø> (?)
#patternfly4	64.78% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2415a94...e5e887b. Read the comment docs.

PatternFly-React preview: https://patternfly-react-pr-3294.surge.sh

Looks like we have two PRs for the same issue?
#3232

👍

How do users see the empty state? Do we need to edit the code to remove rows?

we should probably add a filter to the table and if a filter is set show empty state.

I'm not seeing a spinner. I briefly see the empty state prior to loading the table rows. Is there a way to better see this?

@nicolethoen for some reason I don't seem to be able to access the preview link. It says "project not found."

@dlabrecq @boaz0 I can work to add a way to force view these states.

PatternFly-React preview: https://patternfly-react-pr-3294.surge.sh

Looks great! Built and ran locally...

Thanks for adding the radio buttons

PatternFly-React preview: https://patternfly-react-pr-3294.surge.sh

@nicolethoen finally was able to preview this. Just a couple of changes to request.

Can the header row for the table be shown when the data is being loaded as it does for the empty state?
For the empty state, there should be a 'Clear all filters' action associated with the empty state.

See the core demos here for reference: https://www.patternfly.org/v4/documentation/core/demos/table

@mcarrano those changes should be no problem! There are no filters on the table though, so the 'Clear All Filters' action would be a dead action, so do we still want it? I removed it intentionally for just that reason. If we don't want a dead action, I should probably change the text under the EmptyStateIcon as well so it doesn't reference filters either. But then I'm not sure the best thing for it to say.

That's an interesting question @nicolethoen . I think it's important to have the Clear filters action since this message would only really appear as the result of a filter. But you are right that this would not really do anything in the current demo. Would it take much to add a simple filter to this, like just a Select list with some values?

@mcarrano we have a filterable table demo... would it need to also use the experimental data toolbar to be implemented correctly? seems out of scope for this demo. The only legitimate reason there would be no data to display is if no data is returned from the API. It'd be more of an error.

Yes, that's true @nicolethoen . But this is a slightly different empty state pattern in that case. I will try to set up some time to sync with you on what to put here.

Note: radio buttons were removed - it was decided they were muddying context for the user a little bit. And 'Empty State' was replaced with more of an 'Error state' that is not demonstrated intentionally as part of the demo - but if there is an ever some unknown error retrieving data, it will fail gracefully. Any anyone curious about error handling in this case can look at how it's handled in the code. It was decided this demo is not going to highlight the error state, because it is not an Error State demo, it is a Pagination Table demo.

Wasn't the issue to add "spinner and empty state" to the demo? It's not obvious that these features exist in this example -- I'm not able to see the spinner, nor the empty state. Is there a way we can solve that?

FYI, I had closed #1433 because this issue was to provide empty state functionality.

@dlabrecq I dont think that the Pagination Demo is the place to demonstrate the empty state or error state of a table. It is simply for demonstrating pagination. The other demos don't explicitly demonstrate empty states or error states either.

Before the change, as data was being loaded and paginated, you saw a message that quickly flashed Please wait while loading data before it disappeared. Now you see the spinner instead.

And we've added an error state if no data is retrieved, which should never happen unless there is a connectivity issue - so I added the error state so that any issues that arise will fail gracefully using a Patternfly pattern. So it's not explicitly demonstrable, but is more there as it should if anyone wants to implement Pagination in their table correctly. I think that is following the demo precedent we've established.

These changes were made after talking to @mcarrano and Content strategist Margot

I'll also add that the Filterable Table demo does demonstrate an empty state when all options have been filtered out. That seems to be a more relevant place to demonstrate this. @dlabrecq

I'm just going by what issue #3184 is asking for. If the functionality is not appropriate for the pagination demo, then perhaps we should not modify this particular demo? Or, leave the issue open?

Regardless, my issue is that I don't even see the spinner.

Your machine must be processing the data too fast for you to see it. I almost always see it when I change pagination pages, or when I change the number of items displayed per page. @dlabrecq

@mcarrano would it be appropriate to add a 'View Loading State" checkbox to force the table to stay in a loading state?

How about a timeout that loads the table after a second or two? Then, there is no need for a checkbox

@nicolethoen @dlabrecq I'm good with either of those approaches. It was the inclusion of the empty state here that was causing me the most confusion. I'm wondering if we should have a demo that shows the various types of empty states in context as there are lots of different reasons to get an empty state. Do you think that would be useful?

@dlabrecq I updated the attached issue to reflect the changes made in this PR. And I opened a new issue to document the missing demos. #3330

Any other thoughts @dlabrecq or @mcarrano? Can we get this PR in soon :)

@nicolethoen It makes sense to me to create these in a separate but related demo. I've left a comment to see if we can get that om the roadmap.

Looks good!

Thanks for creating a new issue

Your changes have been released in:

@patternfly/react-catalog-view-extension@1.1.33
@patternfly/react-core@3.122.3
@patternfly/react-docs@4.16.39
@patternfly/react-inline-edit-extension@2.13.4
demo-app-ts@3.11.3
@patternfly/react-table@2.24.36
@patternfly/react-topology@2.11.22
@patternfly/react-virtualized-extension@1.3.35

Thanks for your contribution! 🎉

nicolethoen requested review from boaz0, dlabrecq and mcarrano November 8, 2019 21:00

boaz0 mentioned this pull request Nov 11, 2019

docs(table): add empty state and spinner to pagination table demo #3232

Closed

boaz0 previously approved these changes Nov 11, 2019

View reviewed changes

dlabrecq suggested changes Nov 11, 2019

View reviewed changes

nicolethoen dismissed boaz0’s stale review via c49160e November 12, 2019 14:13

nicolethoen force-pushed the empty_state_to_demo branch from cd04844 to c49160e Compare November 12, 2019 14:13

nicolethoen force-pushed the empty_state_to_demo branch from c49160e to 4390c49 Compare November 12, 2019 15:20

dlabrecq previously approved these changes Nov 12, 2019

View reviewed changes

nicolethoen dismissed dlabrecq’s stale review via 2ff4270 November 13, 2019 19:16

nicolethoen force-pushed the empty_state_to_demo branch from 4390c49 to 2ff4270 Compare November 13, 2019 19:16

mcarrano suggested changes Nov 14, 2019

View reviewed changes

nicolethoen force-pushed the empty_state_to_demo branch from 2ff4270 to 373c12f Compare November 15, 2019 21:12

dlabrecq suggested changes Nov 18, 2019

View reviewed changes

nicolethoen added 5 commits November 18, 2019 16:14

fix(PaginationTableDemo): add Spinner and empty state to demo

484a8a6

add radio toggle to show different display states

d9ec690

changes after discussing with mcarrano and margot

6988a7a

add disclaimer for using experimental spinner

ed5d7cc

add 'View loading state' checkbox

e5e887b

nicolethoen force-pushed the empty_state_to_demo branch from 5514011 to e5e887b Compare November 18, 2019 21:14

mcarrano approved these changes Nov 20, 2019

View reviewed changes

dlabrecq approved these changes Nov 21, 2019

View reviewed changes

dlabrecq merged commit c4cb9f4 into patternfly:master Nov 21, 2019

nicolethoen deleted the empty_state_to_demo branch February 8, 2023 13:46

+                  if (res.length === 0) {
+                    const rows = [{
+                      heightAuto: true,
+                      cells: [
+                        {
+                          props: { colSpan: 8 },
+                          title: (
+                          <Bullseye>
+                            <EmptyState variant={EmptyStateVariant.small}>
+                              <EmptyStateIcon icon={SearchIcon} />
+                              <Title headingLevel="h2" size="lg">
+                                No results found
+                              </Title>
+                              <EmptyStateBody>
+                                No results match the filter criteria. Remove all filters or clear all filters to show results.
+                              </EmptyStateBody>
+                            </EmptyState>
+                          </Bullseye>
+                          )
+                        },
+                      ]
+                    }]

                         </Table>
                       )}
-                      {loading && <center><Title size="3xl">Please wait while loading data</Title></center>}
+                      {loading && <center><Spinner size="xl"/></center>}

fix(PaginationTableDemo): add Spinner and empty state to demo by nicolethoen · Pull Request #3294 · patternfly/patternfly-react · GitHub

Uh oh!

fix(PaginationTableDemo): add Spinner and empty state to demo - #3294

fix(PaginationTableDemo): add Spinner and empty state to demo#3294dlabrecq merged 5 commits intopatternfly:masterpatternfly/patternfly-react:masterfrom nicolethoen:empty_state_to_demonicolethoen/patternfly-react:empty_state_to_demoCopy head branch name to clipboard

Conversation

nicolethoen commented Nov 8, 2019

Uh oh!

codecov-io commented Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

patternfly-build commented Nov 8, 2019

Uh oh!

dlabrecq commented Nov 8, 2019

Uh oh!

boaz0 left a comment

Choose a reason for hiding this comment

Uh oh!

dlabrecq Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

boaz0 Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

dlabrecq Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

mcarrano commented Nov 11, 2019

Uh oh!

nicolethoen commented Nov 12, 2019

Uh oh!

patternfly-build commented Nov 12, 2019

Uh oh!

dlabrecq left a comment • 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!

patternfly-build commented Nov 13, 2019

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

nicolethoen commented Nov 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcarrano commented Nov 14, 2019

Uh oh!

nicolethoen commented Nov 14, 2019

Uh oh!

mcarrano commented Nov 14, 2019

Uh oh!

nicolethoen commented Nov 15, 2019

Uh oh!

dlabrecq left a comment

Choose a reason for hiding this comment

Uh oh!

nicolethoen commented Nov 18, 2019

Uh oh!

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

Uh oh!

Uh oh!

dlabrecq commented Nov 18, 2019

Uh oh!

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

Uh oh!

Uh oh!

dlabrecq commented Nov 18, 2019

Uh oh!

mcarrano commented Nov 18, 2019

Uh oh!

nicolethoen commented Nov 20, 2019

Uh oh!

nicolethoen commented Nov 20, 2019

Uh oh!

mcarrano commented Nov 20, 2019

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

dlabrecq left a comment

fix(PaginationTableDemo): add Spinner and empty state to demo#3294
dlabrecq merged 5 commits into
patternfly:masterfrom
nicolethoen:empty_state_to_demo

codecov-io commented Nov 8, 2019 •
edited
Loading

dlabrecq left a comment •
edited
Loading

nicolethoen commented Nov 14, 2019 •
edited
Loading

nicolethoen commented Nov 18, 2019 •
edited
Loading

nicolethoen commented Nov 18, 2019 •
edited
Loading