Skip to content
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

[App Search] DataPanel & LoadingOverlay component tweaks #94251

Merged
merged 3 commits into from
Mar 10, 2021

Conversation

cee-chen
Copy link
Contributor

Summary

This PR is setup for upcoming Curation work/PRs.

  • DataPanel: fix icons showing unaligned & w/ too much flex space
  • LoadingOverlay: add new loading component w/ overlay for per-panel loading indicators
  • DataPanel: add isLoading prop to show a LoadingOverlay component

Screenshots will be included in the comments below.

Checklist

Accessibility note

Just want to that the below items are potential accessibility point of weaknesses in the app:

  • 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)

Reasons:

  • The LoadingOverlay component prevents mouse clicks over things it covers, but not necessarily keyboard presses (if the user is already focused within the panel)
  • Implementing this from a dialogue / announcement POV is also fairly complex and would require a live region - I'm hoping aria-busy is a sufficient indicator for now
  • Contrast-wise, this likely fails due to opacity shenanigans, but I'm less concerned about this due to the transient nature of the indicator

TL;DR - this isn't a perfect or accessible implementation of a loading indicator and is mostly visual-focused - it's definitely a tech debt item to re-evaluate this at some pointer for better screen reader and keyboard compatibility, possibly via the inert property in the future.

@cee-chen cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 10, 2021
@cee-chen cee-chen requested review from daveyholler and a team March 10, 2021 01:02
Comment on lines +61 to +63
<EuiFlexGroup gutterSize="s" alignItems="center" responsive={false}>
{iconType && (
<EuiFlexItem>
<EuiFlexItem grow={false}>
Copy link
Contributor Author

@cee-chen cee-chen Mar 10, 2021

Choose a reason for hiding this comment

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

This fixes the icon position. It should now look like so:

Note that the icon prop isn't currently in use however, but will be in an upcoming Curation PR.

</EuiFlexItem>
{action && <EuiFlexItem grow={false}>{action}</EuiFlexItem>}
</EuiFlexGroup>
<EuiSpacer />
{children}
{isLoading && <LoadingOverlay />}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what the LoadingOverlay should eventually look like when in use by the Curation view:

This should look very similar to the current loading indicator for the promoted documents section in the standalone UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

My name is Davey and I approve of this loading overlay. 😀

@cee-chen cee-chen force-pushed the curation-panel-setup branch from dadebcb to 5494e76 Compare March 10, 2021 01:10
cee-chen added 2 commits March 9, 2021 17:51
- should have an opacity'd overlay to hide content underneath
- specify z-index
- update CSS to contain LoadingOverlay
- add isLoading prop
@cee-chen cee-chen force-pushed the curation-panel-setup branch from 5494e76 to 7d61dd5 Compare March 10, 2021 01:51
@kibanamachine
Copy link
Contributor

💚 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
enterpriseSearch 2.1MB 2.1MB +1.5KB

History

  • 💔 Build #112348 failed 5494e76ffb69a3f33eb36460fb803a645c6d2825

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

@JasonStoltz JasonStoltz self-assigned this Mar 10, 2021
Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

Code looks good. Not sure if Davey wanted to take a look at this before merging or not. I'm good either way.

Copy link
Contributor

@daveyholler daveyholler left a comment

Choose a reason for hiding this comment

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

🎉

@cee-chen
Copy link
Contributor Author

Thanks y'all! 🎉

@cee-chen cee-chen merged commit dcaa3f6 into elastic:master Mar 10, 2021
@cee-chen cee-chen deleted the curation-panel-setup branch March 10, 2021 16:40
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 10, 2021
* DataPanel: fix icons showing unaligned & w/ too much flex space

* LoadingOverlay: add new loading component w/ overlay

- should have an opacity'd overlay to hide content underneath
- specify z-index

* DataPanel: add flag to display a LoadingOverlay

- update CSS to contain LoadingOverlay
- add isLoading prop
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #94328

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 10, 2021
…4328)

* DataPanel: fix icons showing unaligned & w/ too much flex space

* LoadingOverlay: add new loading component w/ overlay

- should have an opacity'd overlay to hide content underneath
- specify z-index

* DataPanel: add flag to display a LoadingOverlay

- update CSS to contain LoadingOverlay
- add isLoading prop

Co-authored-by: Constance <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants