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

Jump to bottom

[ENG-6116] Sort User Columns for Institutional Dashboard User table #2300

Merged

futa-ikeda merged 23 commits into CenterForOpenScience:feature/insti-dash-improv from Johnetordoff:sort-users-for-inst-dashboard

Sep 9, 2024

Collaborator

Johnetordoff commented Aug 26, 2024 •

edited

Loading

Ticket: [https://openscience.atlassian.net/browse/ENG-6116]
Feature flag: n/a

Purpose

This updates the old flaky institutional dashboard table with a new one that

Summary of Changes

updates templates and css for new sorting arrow
adds new sorting arrow component (Product conversations supported not reusing SortButton)
adds tests to new componet
add to component code to allow toggling the arrow

Screenshot(s)

Side Effects

QA Notes


          make sortable user institution grid for institutions dashboard project

25e44d0

futa-ikeda requested changes

View reviewed changes

Contributor

futa-ikeda left a comment

I think my biggest question is if we want to change the sort-button component for this project, as it is used pretty widely in our codebase. My personal preference is use the sort-button component as it was and have it show two arrows, as I feel that is more informative/less confusing, but I would ask Product team folks

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

lib/osf-components/addon/components/sort-button/template.hbs Outdated Show resolved Hide resolved

lib/osf-components/addon/components/sort-button/template.hbs Outdated Show resolved Hide resolved

lib/osf-components/addon/components/sort-button/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

John Tordoff added 10 commits

August 27, 2024 09:00


          remove rogue and unnecessary classes

e0f5d8e


          add aira label

2795ace


          Merge branch 'feature/insti-dash-improv' of https://github.com/Center…

dece12e

…ForOpenScience/ember-osf-web into sort-users-for-inst-dashboard

* 'feature/insti-dash-improv' of https://github.com/CenterForOpenScience/ember-osf-web:
  [ENG-6078][ENG-6080] Link to past reports (CenterForOpenScience#2296)

# Conflicts:
#	app/institutions/dashboard/-components/institutional-users-list/component.ts


          remove old class

082f25b


          fix sort that was broken with merge

50a0b22


          fix syntax

e0c888a


          add "hidden" columns

80f4e0a


          remove last log action and format dates

b67843b


          syntax fix

e27ab72


          fix arrow styling

1d64e48

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch from e11f7f9 to 1d64e48 Compare

August 27, 2024 19:57

John Tordoff added 2 commits

August 27, 2024 16:31


          humanize bytes

0ad0373


          fix syntax revert and clean-up

840f86d

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch 9 times, most recently from 7e3dac8 to 2619eff Compare

August 28, 2024 21:23

Johnetordoff marked this pull request as ready for review

August 28, 2024 22:27


          revert sort button and create sort arrow component

7911d2e

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch from 2619eff to 7911d2e Compare

August 29, 2024 13:12

futa-ikeda requested changes

View reviewed changes

Contributor

futa-ikeda left a comment

Great work so far! Just a few minor things I see, but I think we are quite close!

app/institutions/dashboard/-components/institutional-users-list/template.hbs

@@ @@ -12,7 +12,7 @@ @@
                   >
                       {{department}}
                   </PowerSelect>
-                  <ContentPlaceholders as |placeholder|>

Contributor

futa-ikeda Aug 30, 2024

It seems this <ContentPlaceholders> is not being used for anymore, so I think we're good to remove this

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated

                                       </th>
-                                      <th local-class='nested-header text-center' colspan='2' width='128'>
-                                          {{t 'institutions.dashboard.users_list.projects'}}
+                                      <th>

Contributor

futa-ikeda Aug 30, 2024

Do we want to add a data-test-header-osf-link or something like that here?

Collaborator Author

Johnetordoff Aug 30, 2024

why not? :) 👍

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated

Comment on lines 47 to 81

+                                      <th>
+                                          <span>{{t 'institutions.dashboard.users_list.private_projects'}}</span>
+                                          <SortArrow @sortBy='private_projects' />
+                                      </th>
+                                      <th data-test-header-private-projects>
+                                          {{t 'institutions.dashboard.users_list.public_registration_count'}}
+                                          <SortArrow @sortBy='public_registration_count' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.private_registration_count'}}
+                                          <SortArrow @sortBy='embargoed_registration_count' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.published_preprint_count'}}
+                                          <SortArrow @sortBy='published_preprint_count' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.public_file_count'}}
+                                          <SortArrow @sortBy='public_file_count' />
                                       </th>
-                                      <th local-class='nested-header text-center' data-test-header-private-projects>
-                                          <img src='/assets/images/global/closed-lock.svg'
-                                              alt={{t 'institutions.dashboard.users_list.private_projects'}}
-                                          >
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.storage_byte_count'}}
+                                          <SortArrow @sortBy='storage_byte_count' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.account_created'}}
+                                          <SortArrow @sortBy='account_created' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.month_last_login'}}
+                                          <SortArrow @sortBy='month_last_login' />
+                                      </th>
+                                      <th>
+                                          {{t 'institutions.dashboard.users_list.month_last_active'}}
+                                          <SortArrow @sortBy='month_last_active' />

Contributor

futa-ikeda Aug 30, 2024

A lot of these <th>'s don't have any data-test attributes. Could you please add these in for tests later?

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated Show resolved Hide resolved

app/institutions/dashboard/-components/institutional-users-list/template.hbs Outdated

Comment on lines 106 to 108

+                                  <td data-test-item-account-created>{{moment-format institutionalUser.accountCreationDate 'MM/DD/YYYY HH:mm:SS'}}</td>
+                                  <td data-test-item-last-login>{{moment-format institutionalUser.monthLastLogin 'MM/DD/YYYY HH:mm:SS'}}</td>
+                                  <td data-test-item-last-active>{{moment-format institutionalUser.monthLastActive 'MM/DD/YYYY HH:mm:SS'}}</td>

Contributor

futa-ikeda Aug 30, 2024

I would ask for Product team for a bit a clarification, but I think for account creation date, we could just do day-level specificity, and for last-login and last-active, we will only show the month level of specificity

Suggested change

      
                                <td data-test-item-account-created>{{moment-format institutionalUser.accountCreationDate 'MM/DD/YYYY HH:mm:SS'}}</td>
          
                                <td data-test-item-last-login>{{moment-format institutionalUser.monthLastLogin 'MM/DD/YYYY HH:mm:SS'}}</td>
          
                                <td data-test-item-last-active>{{moment-format institutionalUser.monthLastActive 'MM/DD/YYYY HH:mm:SS'}}</td>
          
                                <td data-test-item-account-created>{{moment-format institutionalUser.accountCreationDate 'MM/DD/YYYY'}}</td>
          
                                <td data-test-item-last-login>{{moment-format institutionalUser.monthLastLogin 'MM/YYYY'}}</td>
          
                                <td data-test-item-last-active>{{moment-format institutionalUser.monthLastActive 'MM/YYYY'}}</td>

lib/osf-components/addon/components/sort-arrow/component.ts Outdated

		@@ -0,0 +1,25 @@
		import { tagName } from '@ember-decorators/component';
		import Component from '@ember/component';

Contributor

futa-ikeda Aug 30, 2024

Could we change this to a glimmer component? This is a "Ember Classic" Component, and we are trying to move away from these, so for new components I'd like to see us using glimmer components. We can pair on this as there are a number of small changes that are a bit cumbersome to go through in PR comments

There's a quick cheatsheet here with some differences between ember classic and glimmer components that I find myself referring to regularly https://ember-learn.github.io/ember-octane-vs-classic-cheat-sheet/

John Tordoff added 2 commits

August 30, 2024 14:28


          Merge branch 'feature/insti-dash-improv' of https://github.com/Center…

0b15fad

…ForOpenScience/ember-osf-web into sort-users-for-inst-dashboard

* 'feature/insti-dash-improv' of https://github.com/CenterForOpenScience/ember-osf-web:
  Link to reports updates (CenterForOpenScience#2302)


          remove vestigial placeholder

017a771

John Tordoff added 3 commits

August 30, 2024 14:50


          fix account creation sort typo

a7bf1ed


          add test headers for user sort table

4e5c28b


          move sort arrow component to glimmer and re-tool scss

ef3387a

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch 3 times, most recently from c6227d2 to d1b9353 Compare

September 5, 2024 13:04


          update for linting

c32c821

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch 6 times, most recently from 831baf0 to d38d7de Compare

September 5, 2024 16:10


          improve tests and clean-up style

ca31526

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch from d38d7de to ca31526 Compare

September 5, 2024 17:04


          move components for glimmer

a1962b1

futa-ikeda approved these changes

View reviewed changes

Contributor

futa-ikeda left a comment

Just one issue I'd like to see fixed, but that can happen in a later ticket for CSS

lib/osf-components/addon/components/sort-arrow/template.hbs Outdated Show resolved Hide resolved

translations/en-us.yml Outdated Show resolved Hide resolved


          buttonize sort arrows and remove extra translation

352a71d

Johnetordoff force-pushed the sort-users-for-inst-dashboard branch from ecf2df2 to 352a71d Compare

September 6, 2024 10:33

futa-ikeda merged commit 5cc1654 into CenterForOpenScience:feature/insti-dash-improv

9 checks passed

futa-ikeda added this to the 24.09.0 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet