-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add empty data states to desktop dashboard #4156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working as advertised and I can't find anything to comment on 🎉 nice work!
Signed-off-by: Scott Christopherson <scott@chef.io>
Signed-off-by: Scott Christopherson <scott@chef.io>
Signed-off-by: Scott Christopherson <scott@chef.io>
Signed-off-by: Scott Christopherson <scott@chef.io>
Signed-off-by: Scott Christopherson <scott@chef.io>
59ee357
to
eaf20a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but with a couple noted items.
@@ -0,0 +1,2 @@ | |||
<img class="empty-icon" src="assets/img/chart.svg" width="50" alt="" aria-hidden="true" /> | |||
<p class="empty-message">Data not found. Try checking in your Chef Infra Client configuration and give us a moment to load it.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase "checking in" seems... challenging. First I thought it was "checking... inside your config", but then I thought, no, it is probably "checking-in" almost like a git commit, but pretty sure that is not the intent. So I am not at all clear what it is asking the user to do. Could this be rephrased?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msorens I don't have a strong opinion at the moment; I just grabbed the wording from the mockup. I'm certainly open to changing it, adding link(s) to docs, etc. since I'm sure some users could find this ambiguous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kimberly is out til monday, so ill open a card to address the rewording and put it in docs chat and merge this so it can be done!
@@ -71,4 +71,8 @@ export class UnknownDesktopDurationCountsComponent implements OnInit, OnDestroy | |||
return duration; | |||
} | |||
} | |||
|
|||
get hasData(): boolean { | |||
return this.countedDurationItems.some(item => item.count > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following your earlier precedent, may need this guard--or is it guaranteed to exist?
return this.countedDurationItems.some(item => item.count > 0); | |
return this.countedDurationItems && this.countedDurationItems.some(item => item.count > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msorens guaranteed unless the initial reducer state is changed. The other guard only exists because of this filter
in the subscription chain which makes a null
value possible:
automate/components/automate-ui/src/app/modules/desktop/dashboard/dashboard.component.ts
Line 132 in fb331e7
filter(collection => collection.buckets.length > 0)); |
I considered removing the filter
in this PR, but since I didn't add it initially I wasn't 100% sure of other possible consequences for its removal.
🔩 Description: What code changed, and why?
These commits add an 'empty data' state to the desktop dashboard components for when desktop API data is not available.
⛓️ Related Resources
Closes #3335
👍 Definition of Done
Desktop empty states display when desktop API endpoints return no data.
👟 How to Build and Test the Change
a2-dev.test/desktop
✅ Checklist
📷 Screenshots, if applicable