[ENG-6117] Add Filter for Orcid on Instutional Dashboard #2315

Johnetordoff · 2024-09-09T17:02:33Z

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

Purpose

Add the ability to filter users by Orcid status, ensure filter works with sorting.

Summary of Changes

adds slider inputs to user institutional dashboard tab
adds component code to make Orcid filters work
adds template code for layout.

Screenshot(s)

Side Effects

QA Notes

futa-ikeda

I have some concerns with the implementation of the toggle-slider right now. I'm hoping there's a slightly easier way to implement this, so will do a little bit of research to see if there's a potentially more accessible way to implement it.

I had another comment regarding the usage of sass color variables ($color-name) instead of hard-coded hex values in the css files.

futa-ikeda · 2024-09-10T16:27:59Z

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

+        <div local-class='orcid-switch'>
+            <label for='switches-container' local-class='orcid-toggle-label'>{{t 'institutions.dashboard.users_list.has_orcid'}}</label>
+            <div local-class='switches-container'>
+                <input type='radio' id='switchOrcidNo' name='switchPlan' value='No' checked={{not this.hasOrcid}} {{on 'change' (fn this.toggleOrcidFilter false)}} />
+                <input type='radio' id='switchOrcidYes' name='switchPlan' value='Yes' checked={{this.hasOrcid}} {{on 'change' (fn this.toggleOrcidFilter true)}} />
+                <label for='switchOrcidNo'><span></span></label>
+                <label for='switchOrcidYes'><span></span></label>
+                <div local-class='switch-wrapper'>
+                    <div local-class='switch'>
+                        <div>
+                            <button type='button' {{on 'click' (fn this.clickToggleOrcidFilter this.hasOrcid)}}>
+                                <span></span>
+                            </button>
+                        </div>
+                        <div>
+                            <button type='button' {{on 'click' (fn this.clickToggleOrcidFilter this.hasOrcid)}}>
+                                <span></span>
+                            </button>
+                        </div>
+                    </div>
+                </div>
+            </div>


I've got a some concerns with this implementation of the slider. My biggest concern with this right now is the usage of extra <button> elements overlayed on top of the <input> and <label> elements. It feels like we are forcing a type of behavior onto these inputs that may not be appropriate. Could we use a single <input type='checkbox'> instead of a combination of <input type='radio'> with <button>s on top? I think that would simplify the HTML and CSS greatly and make it more accessible

futa-ikeda · 2024-09-10T16:42:53Z

app/institutions/dashboard/-components/institutional-users-list/styles.scss

+    --switches-bg-color: #d3d3d3; /* lightgrey */
+    --switches-label-color: #808080; /* grey */
+    --switch-bg-color: #808080; /* grey */
+    --switch-text-color: #d3d3d3; /* lightgrey */


For CSS colors, we typically try to use our predefined color variables (defined in app/styles/_variables.scss). This prevents us from having too many different "one-off" colors and also helps readability without having to leave comments in the CSS next to the color values (although I appreciate comments in CSS files, generally speaking!). So this could be re-written as something like:

Suggested change

--switches-bg-color: #d3d3d3; /* lightgrey */

--switches-label-color: #808080; /* grey */

--switch-bg-color: #808080; /* grey */

--switch-text-color: #d3d3d3; /* lightgrey */

--switches-bg-color: $color-bg-gray-light;

--switches-label-color: $color-bg-gray;

--switch-bg-color: $color-bg-gray;

--switch-text-color: $color-bg-gray-light;

futa-ikeda · 2024-09-10T16:47:03Z

app/institutions/dashboard/-components/institutional-users-list/styles.scss

+
+/* Hide the radio buttons */
+.switches-container input {
+    visibility: hidden;


I'm a little reluctant to use visibility: hidden; here, as it seems to make it un-tabbable, and that feels like a bit of an accessibility smell to me.

Johnetordoff · 2024-09-10T21:05:53Z

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

@@ -9,10 +9,6 @@ interface SortArrowArgs {

 export default class SortArrow extends Component<SortArrowArgs> {

-    get sortBy() {
-        return this.args.sortBy || 'user_name';


I thought I'd just clean this up here in this PR.

Nice, I think this is a good update. I did notice that in this component file, there's still some references to this deleted property, so instances of this.sortBy should be replaced with this.args.sortBy (line 17, 21)

futa-ikeda

We're close! Just a couple of small things

futa-ikeda · 2024-09-11T15:16:23Z

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

@@ -9,10 +9,6 @@ interface SortArrowArgs {

 export default class SortArrow extends Component<SortArrowArgs> {

-    get sortBy() {
-        return this.args.sortBy || 'user_name';


Nice, I think this is a good update. I did notice that in this component file, there's still some references to this deleted property, so instances of this.sortBy should be replaced with this.args.sortBy (line 17, 21)

futa-ikeda · 2024-09-11T16:10:26Z

app/institutions/dashboard/-components/institutional-users-list/styles.scss

+    opacity: 0;
+    width: 0;
+    height: 0;
+    display: none;


I believe doing display: none on an input will make it impossible to tab to, so I'd like to remove this property for accessibility sake.

futa-ikeda · 2024-09-11T16:12:02Z

app/institutions/dashboard/-components/institutional-users-list/styles.scss

+    transition: background-color 0.4s;
+    background-color: #ccc;
+    cursor: pointer;
+    position: relative;


If we remove display: none from the input as above, we'll have to likely change this to position: absolute and maybe remove display: flex so the slider goes over the input element

futa-ikeda · 2024-09-11T16:12:28Z

app/institutions/dashboard/-components/institutional-users-list/styles.scss

+    margin-left: 3px;
+    border-radius: 50%;
+    transition: transform 0.4s, background-color 0.4s;
+    position: relative;


I think this will have to be position: absolute as well if the input is now shown

futa-ikeda · 2024-09-11T16:20:55Z

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

+                <label for='switches-container' local-class='orcid-toggle-label'>{{t 'institutions.dashboard.users_list.has_orcid'}}</label>
+                <label local-class='switch'>
+                    <input type='checkbox' checked={{this.hasOrcid}} {{on 'change' (fn this.toggleOrcidFilter (not this.hasOrcid))}}>


The input doesn't have a valid label right now, so you will have to add id='switches-container' to the <input> element to associate the "Has ORCID" label to this input

futa-ikeda

🌟

…ForOpenScience/ember-osf-web into filter-dashboard * 'feature/insti-dash-improv' of https://github.com/CenterForOpenScience/ember-osf-web: [ENG-6117] Add Filter for Orcid on Instutional Dashboard (CenterForOpenScience#2315) [ENG-6181] Update search mirage view (CenterForOpenScience#2318) # Conflicts: # app/institutions/dashboard/-components/institutional-users-list/component.ts # app/institutions/dashboard/-components/institutional-users-list/styles.scss # app/institutions/dashboard/-components/institutional-users-list/template.hbs

Johnetordoff force-pushed the filter-orcid branch 14 times, most recently from 5bdd0e5 to affe0a4 Compare September 9, 2024 20:44

add orcid filtering to user list tab of institutional dashboard

b37bddd

Johnetordoff force-pushed the filter-orcid branch from affe0a4 to b37bddd Compare September 9, 2024 20:51

Johnetordoff marked this pull request as ready for review September 9, 2024 20:52

respond to comments and clean-up

f4d9657

futa-ikeda requested changes Sep 10, 2024

View reviewed changes

Johnetordoff force-pushed the filter-orcid branch 4 times, most recently from aef1660 to 8d3ddab Compare September 10, 2024 21:05

Johnetordoff commented Sep 10, 2024

View reviewed changes

Johnetordoff force-pushed the filter-orcid branch from 8d3ddab to bea22fc Compare September 10, 2024 21:08

redo orcid slider css to me more in line with w3

a30a3d1

Johnetordoff force-pushed the filter-orcid branch from bea22fc to a30a3d1 Compare September 10, 2024 21:09

futa-ikeda requested changes Sep 11, 2024

View reviewed changes

respond to CR move slider position to flex

7bd7566

Johnetordoff force-pushed the filter-orcid branch from 313b39a to 7bd7566 Compare September 11, 2024 17:07

futa-ikeda approved these changes Sep 11, 2024

View reviewed changes

futa-ikeda merged commit f6311c4 into CenterForOpenScience:feature/insti-dash-improv Sep 11, 2024
9 checks passed

futa-ikeda added this to the 24.09.0 milestone Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENG-6117] Add Filter for Orcid on Instutional Dashboard #2315

[ENG-6117] Add Filter for Orcid on Instutional Dashboard #2315

Johnetordoff commented Sep 9, 2024 •

edited

Loading

futa-ikeda left a comment

futa-ikeda Sep 10, 2024

futa-ikeda Sep 10, 2024

futa-ikeda Sep 10, 2024

Johnetordoff Sep 10, 2024

futa-ikeda Sep 11, 2024

futa-ikeda left a comment

futa-ikeda Sep 11, 2024

futa-ikeda Sep 11, 2024

futa-ikeda Sep 11, 2024

futa-ikeda Sep 11, 2024

futa-ikeda Sep 11, 2024

futa-ikeda left a comment

[ENG-6117] Add Filter for Orcid on Instutional Dashboard #2315

[ENG-6117] Add Filter for Orcid on Instutional Dashboard #2315

Conversation

Johnetordoff commented Sep 9, 2024 • edited Loading

Purpose

Summary of Changes

Screenshot(s)

Side Effects

QA Notes

futa-ikeda 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

Choose a reason for hiding this comment

futa-ikeda 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

Choose a reason for hiding this comment

futa-ikeda left a comment

Choose a reason for hiding this comment

Johnetordoff commented Sep 9, 2024 •

edited

Loading