Jamie/4343 surface connected services filter in UI #4452

SEAjamieD · 2020-11-25T04:57:17Z

🔩 Description: What code changed, and why?

with the addition of #4339, we have the backend ability to filter service groups by only connected services. This branch adds this ability to the UI

small UX bonus - hide the checkbox for deleting selected services when there all no services to select from

⛓️ Related Resources

Backend Work: #4339

👍 Definition of Done

User is able to filter on connected services in services sidebar.

👟 How to Build and Test the Change

Terminal 1 - in Automate root folder
hab studio enter
build components/automate-ui-devproxy
applications_populate_database

Terminal 2 - cd into components/automate-ui
make serve

Navigate to https://a2-dev.test/applications/service-groups
in the services sidebar, see the "connected" option in the group of filter options
Click on the connected services option to see it filter

✅ Checklist

I have read the CONTRIBUTING document.
Tests added/updated?
Docs added/updated?
All commits have been signed-off for the Developer Certification of Origin.

📷 Screenshots, if applicable

After:

netlify · 2020-11-25T04:57:22Z

Deploy preview for chef-automate ready!

Built with commit 4e57fe0

https://deploy-preview-4452--chef-automate.netlify.app

SEAjamieD · 2020-11-25T20:09:32Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

+        <chef-option class="filter connected" value='connected' (click)="updateHealthFilter('connected')" (keyup.enter)="updateHealthFilter('connected')">
+          <chef-icon class="filter-icon">link</chef-icon>
+          <div class="filter-label">Connected</div>
+          <div class="filter-total">{{ serviceGroupsHealthSummary.total - serviceGroupsHealthSummary.disconnected }}</div>
+        </chef-option>
+        <chef-option class="filter disconnected" value='disconnected' (click)="updateHealthFilter('disconnected')" (keyup.enter)="updateHealthFilter('disconnected')">
+          <chef-icon class="filter-icon"></chef-icon>
+          <div class="filter-label">Disconnected</div>
+          <div class="filter-total">{{ serviceGroupsHealthSummary.disconnected }}</div>
+        </chef-option>


Moved these to the end of the group, per design spec

SEAjamieD · 2020-11-25T20:11:33Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

      </chef-status-filter-group>
    </div>
-    <div class="bulk-action-bar">
+    <div class="bulk-action-bar" [class.hidden]="(services$ | async)?.length === 0" [attr.aria-hidden]="(services$ | async)?.length === 0">


I noticed this checkbox/button was still visible even when there are no services to be selected. This just hides the checkbox/button for UX purposes when theres none to be selected anyways.

SEAjamieD · 2020-11-25T20:12:21Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.scss

+chef-option.filter.unknown {
+  margin-right: 50px; // forces remaining filters into new row
+}
+


per the new design spec for the connected filter option, this is a simple way to force the new row to happen, placing connected and disconnected on their on row.

The other option is to put in an extra div with 0 height and 100% width which effectively works like ye olde <br>, is that true?

My concern here is that you are manipulating the line assuming a constant panel width. If that panel is changed in the future the desired behavior here might be lost. So I would also lean toward something that forces an actual line break.

@danielsdeleo - you absolutely can 👍
@msorens - I see your point, my goal was to add less code, and given that there are no current plans to change anything, simple CSS could give us what we needed.

I'm going to change this to Dan's method above and we'll be all set 👍

SEAjamieD · 2020-11-25T20:13:42Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

+        <chef-option class="filter connected" value='connected' (click)="updateHealthFilter('connected')" (keyup.enter)="updateHealthFilter('connected')">
+          <chef-icon class="filter-icon">link</chef-icon>
+          <div class="filter-label">Connected</div>
+          <div class="filter-total">{{ serviceGroupsHealthSummary.total - serviceGroupsHealthSummary.disconnected }}</div>


the connected count isn't in the API yet, however its just the total minus the disconnected, so we can calculate this on the frontend for the time being.

danielsdeleo

LGTM

danielsdeleo · 2020-11-25T23:07:48Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.scss

+chef-option.filter.unknown {
+  margin-right: 50px; // forces remaining filters into new row
+}
+


The other option is to put in an extra div with 0 height and 100% width which effectively works like ye olde <br>, is that true?

danielsdeleo · 2020-11-25T23:08:16Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

      </chef-status-filter-group>
    </div>
-    <div class="bulk-action-bar">
+    <div class="bulk-action-bar" [class.hidden]="(services$ | async)?.length === 0" [attr.aria-hidden]="(services$ | async)?.length === 0">


msorens

Just a couple notes.

msorens · 2020-11-30T04:15:00Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

@@ -51,9 +46,19 @@
          <div class="filter-label">Unknown</div>
          <div class="filter-total">{{ serviceGroupsHealthSummary.unknown }}</div>
        </chef-option>
+        <chef-option class="filter connected" value='connected' (click)="updateHealthFilter('connected')" (keyup.enter)="updateHealthFilter('connected')">


Would improve the code to tighten the signature of updateHealthFilter to accept only ServiceGroupsFacadeService.allowedStatus (plus "total" with a union type) instead of a general string... but that would require changing allowedStatus to be an enum, I suspect, so may be out of scope for this PR. 🤷

might as well update it while i'm in here. I've updated now and would appreciate your feedback prior to merging

Getting there, but we really only need a single source of truth.
Should be able to eliminate AllowedHealthStatus completely and just use the allowedStatus enum directly (should capitalize to AllowedStatus though). Also, should then delete the entire line health = includes(health, this.allowedHealthStatus) ? health : allowedStatus.TOTAL; in updateHealthFilter; no longer need to check that constraint.

Thanks for the thoughts - I don't think I realized enums could be used as a type signature 👍

msorens · 2020-11-30T04:33:53Z

BTW, I would also suggest setting up some persistent data as you have outlined here but on an acceptance box (e.g. a2-local-inplace-upgrade-acceptance.cd.chef.co) so this could be exercised there as well.

Signed-off-by: seajamied <jdegnan@chef.io>

… from automate Signed-off-by: seajamied <jdegnan@chef.io>

… services Signed-off-by: seajamied <jdegnan@chef.io>

Signed-off-by: seajamied <jdegnan@chef.io>

…calculate it on the frontend for now Signed-off-by: seajamied <jdegnan@chef.io>

… spec Signed-off-by: seajamied <jdegnan@chef.io>

Signed-off-by: seajamied <jdegnan@chef.io>

msorens

Nice!

msorens · 2020-12-01T17:36:30Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

@@ -21,39 +21,45 @@
  <div *ngIf="serviceGroupsId && (serviceGroupsStatus$ | async) === 'loadingSuccess'">
    <div class="status-filter-bar">
      <chef-status-filter-group [value]="selectedHealth" lean>
-        <chef-option class="filter general" value="total" (click)="updateHealthFilter('total')" (keyup.enter)="updateHealthFilter('total')" selected>


Excellent; was hoping you would notice this improvement! 😉

msorens · 2020-12-01T17:50:45Z

components/automate-ui/src/app/page-components/services-sidebar/services-sidebar.component.html

@@ -21,39 +21,45 @@
  <div *ngIf="serviceGroupsId && (serviceGroupsStatus$ | async) === 'loadingSuccess'">
    <div class="status-filter-bar">
      <chef-status-filter-group [value]="selectedHealth" lean>
-        <chef-option class="filter general" value="total" (click)="updateHealthFilter('total')" (keyup.enter)="updateHealthFilter('total')" selected>
+        <chef-option class="filter general" value="total" (click)="updateHealthFilter(allowedStatus.TOTAL)" (keyup.enter)="updateHealthFilter(allowedStatus.TOTAL)" selected>
          <chef-icon class="filter-icon">grain</chef-icon>
          <div class="filter-label">Total</div>


Contrast this with the change above, though just food for thought--you could also do this:

Suggested change

<div class="filter-label">Total</div>

<div class="filter-label">{{allowedStatus.TOTAL | capitalize}}</div>

But I am not really suggesting this; here "Total" is pure presentation so to me is fine as is, whereas on L24 that is code so should be using allowedStatus.TOTAL as you have done.

food for thought, thanks for pointing it out

SEAjamieD commented Nov 25, 2020

View reviewed changes

SEAjamieD requested review from Shadae, tarablack01 and a team November 25, 2020 20:14

SEAjamieD marked this pull request as ready for review November 25, 2020 20:15

danielsdeleo approved these changes Nov 25, 2020

View reviewed changes

msorens approved these changes Nov 30, 2020

View reviewed changes

SEAjamieD requested a review from msorens November 30, 2020 21:01

seajamied added 13 commits November 30, 2020 14:42

Wire up connected filter in service groups sidebar

4cdc55c

Signed-off-by: seajamied <jdegnan@chef.io>

Update chef-ui-library for connected in chef-status-filter and remove…

7fedcf7

… from automate Signed-off-by: seajamied <jdegnan@chef.io>

Update spec and also hide delete connected services when there are no…

dee2da3

… services Signed-off-by: seajamied <jdegnan@chef.io>

Revert hiding checkbox - talk to UX first

72d2d6d

Signed-off-by: seajamied <jdegnan@chef.io>

No connected count exists yet in the summary response so we can just …

3f1af3d

…calculate it on the frontend for now Signed-off-by: seajamied <jdegnan@chef.io>

Force connected and disconnected filters into new row to match design…

1f489f9

… spec Signed-off-by: seajamied <jdegnan@chef.io>

hide select services checkbox when there are none to select

1ab8ffb

Signed-off-by: seajamied <jdegnan@chef.io>

Add aria label for accessibility

1c9f512

Signed-off-by: seajamied <jdegnan@chef.io>

Remove margin CSS and add spacer to markup

357885c

Signed-off-by: seajamied <jdegnan@chef.io>

Update to use stricter typescript

869795a

Signed-off-by: seajamied <jdegnan@chef.io>

Remove some old leftover code from ... i'm not sure where it came from

112698a

Signed-off-by: seajamied <jdegnan@chef.io>

Simplify typescript updates

19119e4

Signed-off-by: seajamied <jdegnan@chef.io>

Update values passed in to updateHealthFilter from template

4e57fe0

Signed-off-by: seajamied <jdegnan@chef.io>

SEAjamieD force-pushed the jamie/4343-connected-services-filter branch from 625ce03 to 4e57fe0 Compare November 30, 2020 22:43

msorens approved these changes Dec 1, 2020

View reviewed changes

SEAjamieD merged commit da1749e into master Dec 1, 2020

SEAjamieD deleted the jamie/4343-connected-services-filter branch December 1, 2020 22:11

msorens changed the title ~~Jamie/4343 connected services filter~~ Jamie/4343 surface connected services filter in UI Jan 7, 2021

mjingle mentioned this pull request Jan 7, 2021

[Chef Automate] Connected/Disconnected Filter docs update chef/chef-web-docs#2848

Closed

msorens added the acceptance: verified label Jan 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jamie/4343 surface connected services filter in UI #4452

Jamie/4343 surface connected services filter in UI #4452

SEAjamieD commented Nov 25, 2020 •

edited

Loading

netlify bot commented Nov 25, 2020 •

edited

Loading

SEAjamieD Nov 25, 2020

SEAjamieD Nov 25, 2020

danielsdeleo Nov 25, 2020

SEAjamieD Nov 25, 2020

danielsdeleo Nov 25, 2020

msorens Nov 30, 2020

SEAjamieD Nov 30, 2020

SEAjamieD Nov 25, 2020

danielsdeleo left a comment

danielsdeleo Nov 25, 2020

danielsdeleo Nov 25, 2020

msorens left a comment

msorens Nov 30, 2020

SEAjamieD Nov 30, 2020

msorens Nov 30, 2020

SEAjamieD Nov 30, 2020

msorens commented Nov 30, 2020

msorens left a comment

msorens Dec 1, 2020

msorens Dec 1, 2020

SEAjamieD Dec 1, 2020

	<div class="filter-label">Total</div>
	<div class="filter-label">{{allowedStatus.TOTAL \| capitalize}}</div>

Jamie/4343 surface connected services filter in UI #4452

Jamie/4343 surface connected services filter in UI #4452

Conversation

SEAjamieD commented Nov 25, 2020 • edited Loading

🔩 Description: What code changed, and why?

⛓️ Related Resources

👍 Definition of Done

👟 How to Build and Test the Change

✅ Checklist

📷 Screenshots, if applicable

netlify bot commented Nov 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielsdeleo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msorens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msorens commented Nov 30, 2020

msorens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SEAjamieD commented Nov 25, 2020 •

edited

Loading

netlify bot commented Nov 25, 2020 •

edited

Loading