Skip to content

[SharedUX] Add loading indicator to NoDataPage#132272

Merged
majagrubic merged 6 commits intoelastic:mainfrom
majagrubic:no-data-loading
May 18, 2022
Merged

[SharedUX] Add loading indicator to NoDataPage#132272
majagrubic merged 6 commits intoelastic:mainfrom
majagrubic:no-data-loading

Conversation

@majagrubic
Copy link
Contributor

@majagrubic majagrubic commented May 16, 2022

Summary

When we started adding the NoDataPage in Kibana, it became obvious we were missing the loading state of the component. The component fires two requests: hasEsData and hasUserDataView. If the second one finishes before the first, there will be an ugly transition between the two states, as shown in the GIF below (notice how the no data component is shown first. Then it disappears and "no data views" appears, which should be the end state):
transition_fix

This PR adds a simple loading indicator that should be shown until both requests have finished. This way the component will render immediately the state it needs. Observe how the loading indicator is shown first and then "no data views" components shows immediately:

after_gif

I've just added a simple EuiLoadingSpinner to be shown while the requests are running. I don't think we need a more sophisticated way of showing a loading state here @cchaos?

Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
    - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
  • This was checked for cross-browser compatibility

For maintainers

@majagrubic majagrubic requested review from a team and cchaos May 16, 2022 18:25
@majagrubic majagrubic marked this pull request as ready for review May 16, 2022 20:09
@majagrubic majagrubic added Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.3.0 release_note:skip Skip the PR/issue when compiling release notes labels May 16, 2022

export const KibanaNoDataPage = ({ onDataViewCreated, noDataConfig }: Props) => {
const { hasESData, hasUserDataView } = useData();
const [hasFinishedLoading, setHasFinishedLoading] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use the isLoading flag for this. Then on L38 you don't need a negation. Seems more direct to me.

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

…ta_page.tsx

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataViewEditor 171.9KB 172.1KB +163.0B
discover 529.9KB 530.1KB +164.0B
lens 1.2MB 1.2MB +163.0B
securitySolution 5.0MB 5.0MB +164.0B
total +654.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woot! Nice and centered, now if we can just do the same for withSuspense() 😼

@majagrubic
Copy link
Contributor Author

Thanx for looking into it!

@majagrubic majagrubic merged commit 1343ef3 into elastic:main May 18, 2022
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 18, 2022
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants