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.

Sign up for GitHub

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

Improved Dashboard Error Feedback #7427

Merged
merged 9 commits into from
Dec 1, 2021
Merged

Improved Dashboard Error Feedback #7427

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0cb33e9
make loading state more obvious
pauldambra Nov 30, 2021
142cabd
error state if the API fails
pauldambra Nov 30, 2021
7ec623e
better handle errors in whole dashboard or individual dashboard items
pauldambra Nov 30, 2021
47ac113
take error knowledge out of the empty state component
pauldambra Nov 30, 2021
c5e4465
improved name for skipping insight error details
pauldambra Nov 30, 2021
75aacbc
make a props item optional
pauldambra Nov 30, 2021
e092085
Update frontend/src/styles/global.scss
pauldambra Nov 30, 2021
96835fa
dashboard item error state matches container
pauldambra Nov 30, 2021
a89881a
make background transparent, not spinner
pauldambra Nov 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export interface InsightErrorStateProps {

export function InsightErrorState({ excludeDetail, title }: InsightErrorStateProps): JSX.Element {
return (
<div className="insight-empty-state error">
<div className={clsx(['insight-empty-state', 'error', { 'match-container': excludeDetail }])}>
<div className="empty-state-inner">
<div className="illustration-main">
<IllustrationDanger />
Expand Down
40 changes: 40 additions & 0 deletions frontend/src/scenes/insights/Insight.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,46 @@ $funnel_canvas_background: #fafafa;
}
}

.dashboard-item {
.insight-empty-state {
&.match-container {
background-color: transparent;
}
}

&.purple {
.insight-empty-state {
&.match-container {
&.error {
.illustration-main {
color: $text_light;
}

h2 {
color: $text_light;
}
}
}
}
}

&:not(.purple) {
Comment on lines +324 to +340
Copy link
Member

Choose a reason for hiding this comment

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

Hm, not sure about giving just purple special handling here - maybe all non-white should have this styling?

Copy link
Member Author

Choose a reason for hiding this comment

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

I might pick up the storybook as described above then. Can check which colours do or don't work with white text then

.insight-empty-state {
&.match-container {
&.error {
.illustration-main {
color: $danger;
}

h2 {
color: $danger;
}
}
}
}
}
}

.trends-insights-container {
position: relative;
min-height: min(calc(90vh - 16rem), 36rem);
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/styles/global.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,10 +786,9 @@ body {
position: absolute;
top: 0;
left: 0;
background: $bg_charcoal;
background: transparentize($bg_charcoal, 0.5);
text-align: center;
min-height: 6rem;
opacity: 0.7;
z-index: $z_content_overlay;
display: flex;
align-items: center;
Expand Down