Feature/improve sorting ux #1541 #1578

kaperoo · 2023-09-01T10:47:37Z

Improved sorting UX

This PR implements improved sorting UX.
Changes/ New features:

Each sortable (but not sorted) column now has an 'SortIcon' icon so users know it is clickable.
When shift key is pressed (for multi sort), all Sort Icons change to Add Icons
Single column sort is now the default
Multi column sort is applied when columns are clicked with shift key down
Sorting in the card view works in a similar way to table view, except it doesn't display 'SortIcon' (please let me know if it should).

Testing instructions

Had to modify multiple existing unit and e2e tests to account for new way of multi column sorting.
Additionally, added tests checking for icon changes.

Agile board tracking

connect to #1541

codecov · 2023-09-01T14:15:43Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (48e434d) 96.19% compared to head (848d799) 96.26%.
Report is 17 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1578      +/-   ##
===========================================
+ Coverage    96.19%   96.26%   +0.06%     
===========================================
  Files          161      161              
  Lines         6863     6989     +126     
  Branches      2135     2179      +44     
===========================================
+ Hits          6602     6728     +126     
  Misses         240      240              
  Partials        21       21

Flag	Coverage Δ
common	`95.61% <100.00%> (+0.09%)`	⬆️
dataview	`96.94% <ø> (+0.08%)`	⬆️
download	`96.34% <100.00%> (+0.02%)`	⬆️
search	`96.47% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
packages/datagateway-common/src/api/index.tsx	`96.73% <100.00%> (+0.02%)`	⬆️
...datagateway-common/src/card/cardView.component.tsx	`71.62% <100.00%> (+2.35%)`	⬆️
...src/table/headerRenderers/dataHeader.component.tsx	`97.56% <100.00%> (+1.56%)`	⬆️
...s/datagateway-common/src/table/table.component.tsx	`94.69% <100.00%> (+0.57%)`	⬆️
...d/src/downloadCart/downloadCartTable.component.tsx	`94.52% <100.00%> (+0.07%)`	⬆️
...nloadStatus/adminDownloadStatusTable.component.tsx	`99.32% <100.00%> (+<0.01%)`	⬆️
...c/downloadStatus/downloadStatusTable.component.tsx	`94.70% <100.00%> (+0.07%)`	⬆️

... and 26 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

louise-davies · 2023-09-12T08:37:54Z

@kevinphippsstfc do you have any thoughts about the sort icon?

louise-davies · 2023-09-14T09:42:39Z

packages/datagateway-common/src/card/cardView.component.tsx

@@ -233,20 +259,20 @@ const CardView = (props: CardViewProps): React.ReactElement => {
  //defaultSort has been provided
  React.useEffect(() => {
    if (title.defaultSort !== undefined && sort[title.dataKey] === undefined)
-      onSort(title.dataKey, title.defaultSort, 'replace');
+      onSort(title.dataKey, title.defaultSort, 'replace', false);


Minor comment, but since you specified shiftDown?: boolean as optional, you could leave out explicitly specifying false here. Would have meant less changes in the tests - but no point removing it now, just an observation

louise-davies · 2023-09-14T09:48:27Z

packages/datagateway-common/src/card/cardView.component.tsx

+                                  <TableSortLabel
+                                    active={true}
+                                    direction={sort[s.dataKey]}
+                                    // Set tabindex to -1 to prevent button focus
+                                    tabIndex={-1}
+                                  >
+                                    {s.dataKey in sort && sort[s.dataKey]}
+                                  </TableSortLabel>
+                                ) : shiftDown ? (
+                                  <TableSortLabel
+                                    active={true}
+                                    direction={sort[s.dataKey]}
+                                    // Set tabindex to -1 to prevent button focus
+                                    tabIndex={-1}
+                                    IconComponent={AddIcon}
+                                  >
+                                    {s.dataKey in sort && sort[s.dataKey]}
+                                  </TableSortLabel>


You could simply & reduce code repetition here by removing the ternary checks and changing TableSortLabel to:

<TableSortLabel active={s.dataKey in sort} direction={sort[s.dataKey]} // Set tabindex to -1 to prevent button focus tabIndex={-1} IconComponent={!(s.dataKey in sort) && shiftDown ? AddIcon : undefined} > {s.dataKey in sort && sort[s.dataKey]} </TableSortLabel>

louise-davies · 2023-09-14T09:57:03Z

packages/datagateway-common/src/table/headerRenderers/dataHeader.component.tsx

+    dataKey in sort ? (
+      <TableSortLabel
+        className={'tour-dataview-sort'}
+        active={true}
+        direction={currSortDirection}
+        onClick={(event) =>
+          onSort(dataKey, nextSortDirection, 'push', event.shiftKey)
+        }
+      >
+        <Typography noWrap sx={{ fontSize: 'inherit', lineHeight: 'inherit' }}>
+          {label}
+        </Typography>
+      </TableSortLabel>
+    ) : (
+      <TableSortLabel
+        className={'tour-dataview-sort'}
+        active={true}
+        direction={'desc'}
+        onClick={(event) => {
+          onSort(dataKey, nextSortDirection, 'push', event.shiftKey);
+          setHover(false);
+        }}
+        onMouseEnter={() => setHover(true)}
+        onMouseLeave={() => setHover(false)}
+        IconComponent={hover ? ArrowUpwardIcon : shiftDown ? AddIcon : SortIcon}
+        sx={{
+          transition: 'opacity 0.5s',
+          opacity: hover ? 0.7 : 1,
+        }}


Again, some code repetition here. We can reduce this by only adding the extra props when dataKey in sort is false, e.g.

<TableSortLabel className={'tour-dataview-sort'} active={true} direction={!(dataKey in sort) ? 'desc' : currSortDirection} onClick={(event) => { onSort(dataKey, nextSortDirection, 'push', event.shiftKey); if (!(dataKey in sort)) setHover(false); }} {...(!(dataKey in sort) ? { onMouseEnter: () => setHover(true), onMouseLeave: () => setHover(false), IconComponent: hover ? ArrowUpwardIcon : shiftDown ? AddIcon : SortIcon, sx: { transition: 'opacity 0.5s', opacity: hover ? 0.7 : 1, }, } : {})} > <Typography noWrap sx={{ fontSize: 'inherit', lineHeight: 'inherit' }}> {label} </Typography> </TableSortLabel>

louise-davies

Works well, just some minor code styling comments. I like that pressing shift changes the sort buttons to add buttons to represent the different action possible :)

louise-davies

Comments have been addressed - just want to wait for @kevinphippsstfc to take a look before we merge

louise-davies

Kevin suggested we add hover tooltips to the sort indicators to be explicit that they're prompting the users to sort :)

kaperoo · 2023-10-11T13:52:35Z

Added requested hover tooltips to sort indicators.

When hovered:

When hovered with shift down:

Let me know if the tooltips should be more descriptive (i.e. "Sort asc") :)

The tooltips appearing slow the tests down so that they risk timing out

…/ral-facilities/datagateway into feature/improve-sorting-ux-#1541

tests keep timing out so this might help?

louise-davies

LGTM and Kevin was happy with the UI & the tooltips are working well

kaperoo added 2 commits September 1, 2023 11:40

Added shift+click sorting #1541

70a8178

Code cleanup #1541

d1294dd

kaperoo linked an issue Sep 1, 2023 that may be closed by this pull request

Improve sorting UX #1541

Closed

3 tasks

Sorting unit test fix #1541

88d2702

kaperoo added 12 commits September 4, 2023 09:51

Shift-click sorting for download tables #1541

a28a6bd

Fixed sorting in card view #1541

e03740d

changed the way shift is passed to onSort #1541

374c857

Fix issues with download e2e tests #1541

0586bd4

changed the way shiftDown is passed in cardview #1541

ca20c22

fix to the single column sort test in admin download #1541

52d0395

Fixed dataview e2e tests #1541

72be066

Fix dataview e2e tests 2 #1541

e94c94a

Improved code coverage #1541

76f26b8

Merge branch 'develop' into feature/improve-sorting-ux-#1541

f86c192

Adjusted + added e2e tests for multiple column sorting #1541

79e4336

fix e2e test for adminDownloadDtatus #1541

77f3972

kaperoo marked this pull request as ready for review September 12, 2023 08:22

kaperoo requested a review from louise-davies September 12, 2023 08:23

Merge branch 'develop' into feature/improve-sorting-ux-#1541

c6bd356

louise-davies reviewed Sep 14, 2023

View reviewed changes

Reduce code repetition

be9ef8b

kaperoo requested a review from louise-davies September 25, 2023 09:43

louise-davies reviewed Sep 27, 2023

View reviewed changes

louise-davies requested changes Oct 11, 2023

View reviewed changes

add hover tooltips to sort indicators

c9162f3

kaperoo and others added 7 commits October 12, 2023 12:47

Hopefully fixed failing tests

7553763

updated snapshots

7492fb3

fix failing e2e test

95cf7fc

#1541 - use skipHover to avoid triggering sort tooltips in long tests

b6bca39

The tooltips appearing slow the tests down so that they risk timing out

Merge branch 'feature/improve-sorting-ux-#1541' of https://github.com…

0043575

…/ral-facilities/datagateway into feature/improve-sorting-ux-#1541

use skipHover to fix another test #1541

7fe8811

#1541 - add delay null to downloadStatusTable userEvent

848d799

tests keep timing out so this might help?

kaperoo requested a review from louise-davies October 13, 2023 10:02

louise-davies approved these changes Oct 25, 2023

View reviewed changes

kaperoo merged commit 6923aca into develop Oct 25, 2023
9 checks passed

louise-davies deleted the feature/improve-sorting-ux-#1541 branch November 3, 2023 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/improve sorting ux #1541 #1578

Feature/improve sorting ux #1541 #1578

kaperoo commented Sep 1, 2023 •

edited

Loading

codecov bot commented Sep 1, 2023 •

edited

Loading

louise-davies commented Sep 12, 2023

louise-davies Sep 14, 2023

louise-davies Sep 14, 2023

louise-davies Sep 14, 2023

louise-davies left a comment •

edited

Loading

louise-davies left a comment

louise-davies left a comment

kaperoo commented Oct 11, 2023

louise-davies left a comment

Feature/improve sorting ux #1541 #1578

Feature/improve sorting ux #1541 #1578

Conversation

kaperoo commented Sep 1, 2023 • edited Loading

Improved sorting UX

Testing instructions

Agile board tracking

codecov bot commented Sep 1, 2023 • edited Loading

Codecov Report

louise-davies commented Sep 12, 2023

louise-davies Sep 14, 2023

Choose a reason for hiding this comment

louise-davies Sep 14, 2023

Choose a reason for hiding this comment

louise-davies Sep 14, 2023

Choose a reason for hiding this comment

louise-davies left a comment • edited Loading

Choose a reason for hiding this comment

louise-davies left a comment

Choose a reason for hiding this comment

louise-davies left a comment

Choose a reason for hiding this comment

kaperoo commented Oct 11, 2023

louise-davies left a comment

Choose a reason for hiding this comment

kaperoo commented Sep 1, 2023 •

edited

Loading

codecov bot commented Sep 1, 2023 •

edited

Loading

louise-davies left a comment •

edited

Loading