Improved Dashboard Error Feedback #7427
Conversation
frontend/src/styles/global.scss
Outdated
| .loading-overlay { | ||
| width: 100%; | ||
| height: 100%; | ||
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| background: $bg_charcoal; | ||
| text-align: center; | ||
| min-height: 6rem; | ||
| opacity: 0.7; | ||
| z-index: $z_content_overlay; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
|
|
||
| &.over-table { | ||
| display: block; | ||
| background: rgba(0, 0, 0, 0.1); | ||
| td { | ||
| display: block; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The file is listed as deprecated so I thought better not to edit
| if (showErrorMessage) { | ||
| return <InsightErrorState /> | ||
| if (showErrorMessage || receivedErrorFromAPI) { | ||
| return <InsightErrorState excludeDetail={true} /> |
There was a problem hiding this comment.
InsightErrorState's gray background makes sense in the larger size (excludeDetail={false}) but here it looks kind of sloppy:
I'm going to rely on @clarkus's skills to suggest the better way though.
There was a problem hiding this comment.
These errors states do look a little cramped. If we retain the gray background, we should pad the container a bit more. If it's feasible to drop the gray background and reuse colors from the dashboard item color styles, then we wouldn't need to change padding. Put together a quick example:
There was a problem hiding this comment.
Solid improvements! Left some examples to guide feedback, but I don't see anything blocking from a UX point of view. Nice job, @pauldambra
Co-authored-by: Michael Matloka <dev@twixes.com>
spinner with just background opaquified
error state matching the container
|
|
Not a review, but a comment that it would be awesome to (somehow? eventually?) see all these different states, with all the available colors somewhere on a storybook. I started something like this in #7017, but it got stalled. Here's the storybook page as it is in that PR. The next steps is to add all the different types of charts with some real-ish data by copying a few API responses into json files. The step after that is to add other stories where all the charts show a specific state (loading, error, timeout, etc). I'd be happy if someone could pick that up. 🤞 |
| &.purple { | ||
| .insight-empty-state { | ||
| &.match-container { | ||
| &.error { | ||
| .illustration-main { | ||
| color: $text_light; | ||
| } | ||
|
|
||
| h2 { | ||
| color: $text_light; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| &:not(.purple) { |
There was a problem hiding this comment.
Hm, not sure about giving just purple special handling here - maybe all non-white should have this styling?
There was a problem hiding this comment.
I might pick up the storybook as described above then. Can check which colours do or don't work with white text then
Changes
Closes #6285
This introduces three changes
before loading change
after loading change
whole dashboard error
before
after
individual item errors
#### before
after
How did you test this code?
with some unit tests and manual testing
in order to test manually the easiest route is to load a dashboard then run
redis-cli KEYS "*cache*" | xargs redis-cli DELso the cache for the items is removed and then insert a throw in either thedashboardLogic > loaders > allItemstodashboardLogic > refreshAllDashboardItems > fetchItemFunctionsdepending on if you want to error on all items or individual items respectively then either refresh the page or click to refresh the dashboard