From ed86aaaea2503a6693e71fa0ed600bb9d561e0e3 Mon Sep 17 00:00:00 2001
From: Brian Pilati <brianpilati@cos.io>
Date: Tue, 8 Oct 2024 10:55:33 -0500
Subject: [PATCH] Minor updates for comments

---
 .../chart-kpi/component-test.ts               |  6 ++-
 .../chart-kpi-wrapper/chart-kpi/component.ts  |  4 +-
 .../chart-kpi-wrapper/chart-kpi/styles.scss   | 14 ++----
 .../chart-kpi-wrapper/chart-kpi/template.hbs  | 43 +++++++++++--------
 .../chart-kpi-wrapper/component.ts            |  2 +-
 .../total-count-kpi-wrapper/component-test.ts |  6 +--
 translations/en-us.yml                        |  2 +
 7 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component-test.ts b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component-test.ts
index f06fc4053a..9c74b86fce 100644
--- a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component-test.ts
+++ b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component-test.ts
@@ -1,4 +1,4 @@
-import { click, render } from '@ember/test-helpers';
+import { click, pauseTest, render } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { setupMirage } from 'ember-cli-mirage/test-support';
 import { setupIntl } from 'ember-intl/test-support';
@@ -47,7 +47,9 @@ module('Integration | institutions | dashboard | -components | chart-kpi', hooks
 
         // Finally the expanded data is not visible
         assert.dom('[data-test-expansion-data]')
-            .doesNotExist('The expansion data is not visible');
+            .hasStyle({display: 'none'});
+        // eslint-disable-next-line ember/no-pause-test
+        await pauseTest();
 
     });
 
diff --git a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component.ts b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component.ts
index dce4269a42..711a744af1 100644
--- a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component.ts
+++ b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/component.ts
@@ -20,7 +20,7 @@ interface DataModel {
 export default class ChartKpi extends Component<KPIChartWrapperArgs> {
     @service intl!: Intl;
 
-    @tracked expanded = false;
+    @tracked collapsed = true;
     @tracked expandedData = [] as DataModel[];
 
     /**
@@ -107,6 +107,6 @@ export default class ChartKpi extends Component<KPIChartWrapperArgs> {
 
     @action
     public toggleExpandedData() {
-        this.expanded = !this.expanded;
+        this.collapsed = !this.collapsed;
     }
 }
diff --git a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/styles.scss b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/styles.scss
index c79f45e3d0..4b7974f775 100644
--- a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/styles.scss
+++ b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/styles.scss
@@ -55,16 +55,6 @@
                 flex-direction: row;
                 justify-content: center;
                 align-items: center;
-
-                .facet-expand-button {
-                    display: flex;
-                    justify-content: space-between;
-                    align-items: center;
-
-                    &:active {
-                        box-shadow: none;
-                    }
-                }
             }
         }
         
@@ -75,6 +65,10 @@
             justify-content: center;
             align-items: flex-start;
 
+            &.collapsed {
+                display: none;
+            }
+
             .data-list {
                 list-style: none;
                 margin: 0;
diff --git a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/template.hbs b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/template.hbs
index c3aab8114e..c62c58f81f 100644
--- a/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/template.hbs
+++ b/app/institutions/dashboard/-components/chart-kpi-wrapper/chart-kpi/template.hbs
@@ -15,23 +15,32 @@
         </div>
     </div>
     <div local-class='bottom-container'>
-        <div local-class='title-container'>
-            <div data-test-chart-title local-class='title'>{{@data.title}}</div>
-            <div local-class='button-container'>
-                <Button
-                    data-analytics-name='Expand additional data'
-                    data-test-expand-additional-data
-                    @layout='fake-link'
-                    aria-expanded={{this.expanded}}
-                    {{on 'click' this.toggleExpandedData}}
-                >
-                    <FaIcon data-test-toggle-icon @icon={{if this.expanded 'caret-up' 'caret-down'}} />
-                </Button>
+        {{#let (unique-id 'expanded-data') as |expandedDataId|}}
+            <div local-class='title-container'>
+                <div data-test-chart-title local-class='title'>{{@data.title}}</div>
+                <div local-class='button-container'>
+                    <Button
+                        data-analytics-name='Expand additional data'
+                        data-test-expand-additional-data
+                        @layout='fake-link'
+                        aria-controls={{expandedDataId}}
+                        aria-expanded={{this.expanded}}
+                        aria-label={{if this.expanded
+                        (t 'institutions.dashboard.kpi-chart.close-expanded-data')
+                        (t 'institutions.dashboard.kpi-chart.open-expanded-data')
+                        }}
+                        {{on 'click' this.toggleExpandedData}}
+                    >
+                        <FaIcon data-test-toggle-icon @icon={{if this.collapsed 'caret-down' 'caret-up'}} />
+                    </Button>
+                </div>
             </div>
-        </div>
-        {{#if this.expanded }}
-            <div local-class='expanded-data-container'>
-                <ul local-class='data-list' data-test-expansion-data>
+            <div 
+                local-class='expanded-data-container {{if this.collapsed 'collapsed'}}'
+                data-test-expansion-data
+                id={{expandedDataId}}
+            >
+                <ul local-class='data-list'>
                     {{#each this.expandedData as |data index |}}
                         <li
                             local-class='data-container'
@@ -55,6 +64,6 @@
                     {{/each}}
                 </ul>
             </div>
-        {{/if}}
+        {{/let}}
     </div>
 </div>
\ No newline at end of file
diff --git a/app/institutions/dashboard/-components/chart-kpi-wrapper/component.ts b/app/institutions/dashboard/-components/chart-kpi-wrapper/component.ts
index 268508bcc8..c3ec684cf4 100644
--- a/app/institutions/dashboard/-components/chart-kpi-wrapper/component.ts
+++ b/app/institutions/dashboard/-components/chart-kpi-wrapper/component.ts
@@ -107,7 +107,7 @@ export default class ChartKpiWrapperComponent extends Component<TotalCountChartW
     private calculateUsersByDepartment(departmentMetrics: InstitutionDepartmentModel[]): ChartDataModel[] {
         const departmentData = [] as ChartDataModel[];
 
-        departmentMetrics.map((metric: InstitutionDepartmentModel ) => {
+        departmentMetrics.forEach((metric: InstitutionDepartmentModel ) => {
             departmentData.push(
                 {
                     label: metric.name,
diff --git a/app/institutions/dashboard/-components/total-count-kpi-wrapper/component-test.ts b/app/institutions/dashboard/-components/total-count-kpi-wrapper/component-test.ts
index a4a7fffd75..07d1f43435 100644
--- a/app/institutions/dashboard/-components/total-count-kpi-wrapper/component-test.ts
+++ b/app/institutions/dashboard/-components/total-count-kpi-wrapper/component-test.ts
@@ -41,7 +41,7 @@ module('Integration | institutions | dashboard | -components | total-count-kpi-w
             .hasText('10 Total Users');
 
         assert.dom('[data-test-total-count-kpi="0"] [data-test-kpi-icon]')
-            .hasAttribute('data-icon', 'building');
+            .hasAttribute('data-icon', 'users');
 
         // And the second total kpi is tested
         assert.dom('[data-test-total-count-kpi="1"]')
@@ -51,7 +51,7 @@ module('Integration | institutions | dashboard | -components | total-count-kpi-w
             .hasText('20 OSF Public and Private Projects');
 
         assert.dom('[data-test-total-count-kpi="1"] [data-test-kpi-icon]')
-            .hasAttribute('data-icon', 'atom');
+            .hasAttribute('data-icon', 'flask');
 
         // And the third total kpi is tested
         assert.dom('[data-test-total-count-kpi="2"]')
@@ -61,7 +61,7 @@ module('Integration | institutions | dashboard | -components | total-count-kpi-w
             .hasText('100 OSF Registrations');
 
         assert.dom('[data-test-total-count-kpi="2"] [data-test-kpi-icon]')
-            .hasAttribute('data-icon', 'flag');
+            .hasAttribute('data-icon', 'archive');
 
         // And the fourth total kpi is tested
         assert.dom('[data-test-total-count-kpi="3"]')
diff --git a/translations/en-us.yml b/translations/en-us.yml
index a7995572fb..abe3b2e2b0 100644
--- a/translations/en-us.yml
+++ b/translations/en-us.yml
@@ -832,6 +832,8 @@ institutions:
             registrations: 'OSF Registrations'
             preprints: 'OSF Preprints'
         kpi-chart:
+            open-expanded-data: 'Expand Additionnal Data'
+            close-expanded-data: 'Collapse Additionnal Data'
             users-by-department: 'Total Users by Department'
             public-vs-private-projects:
                 title: 'Public vs Private Projects'