Skip to content

Commit

Permalink
Merge pull request #1578 from ral-facilities/feature/improve-sorting-…
Browse files Browse the repository at this point in the history 
…ux-#1541

Feature/improve sorting ux #1541
  • Loading branch information
@kaperoo
kaperoo authored Oct 25, 2023
2 parents 719ef05 + 848d799 commit 6923aca
Show file tree
Hide file tree
Showing 40 changed files with 1,109 additions and 270 deletions.
42 changes: 42 additions & 0 deletions packages/datagateway-common/src/api/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ describe('generic api functions', () => {
afterEach(() => {
jest.restoreAllMocks();
jest.resetModules();
window.history.pushState({}, 'Test', '/');
});

describe('useSort', () => {
Expand Down Expand Up @@ -398,6 +399,47 @@ describe('generic api functions', () => {
search: '?',
});
});

it('returns callback that, when called without shift modifier, replaces sort with the new one', () => {
window.history.pushState(
{},
'Test',
'?sort=%7B%22name%22%3A%22asc%22%7D'
);
const { result } = renderHook(() => useSort(), {
wrapper,
});

act(() => {
result.current('title', 'asc', 'push', false);
});

expect(pushSpy).toHaveBeenCalledWith({
search: `?sort=${encodeURIComponent('{"title":"asc"}')}`,
});
});

it('returns callback that, when called with shift modifier, appends new sort to the existing one', () => {
window.history.pushState(
{},
'Test',
'?sort=%7B%22name%22%3A%22asc%22%7D'
);

const { result } = renderHook(() => useSort(), {
wrapper,
});

act(() => {
result.current('title', 'asc', 'push', true);
});

console.log(history.location.search);

expect(pushSpy).toHaveBeenCalledWith({
search: `?sort=${encodeURIComponent('{"name":"asc","title":"asc"}')}`,
});
});
});

describe('usePushFilter', () => {
Expand Down
27 changes: 18 additions & 9 deletions packages/datagateway-common/src/api/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,25 +260,34 @@ export const getApiParams = (
export const useSort = (): ((
sortKey: string,
order: Order | null,
updateMethod: UpdateMethod
updateMethod: UpdateMethod,
shiftDown?: boolean
) => void) => {
const { push, replace } = useHistory();

return React.useCallback(
(
sortKey: string,
order: Order | null,
updateMethod: UpdateMethod
updateMethod: UpdateMethod,
shiftDown?: boolean
): void => {
let query = parseSearchToQuery(window.location.search);
if (order !== null) {
query = {
...query,
sort: {
...query.sort,
[sortKey]: order,
},
};
query = shiftDown
? {
...query,
sort: {
...query.sort,
[sortKey]: order,
},
}
: {
...query,
sort: {
[sortKey]: order,
},
};
} else {
// if order is null, user no longer wants to sort by that column so remove column from sort state
const { [sortKey]: order, ...rest } = query.sort;
Expand Down
66 changes: 56 additions & 10 deletions packages/datagateway-common/src/card/cardView.component.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ describe('Card View', () => {
await user.click(
await screen.findByRole('button', { name: 'Sort by TITLE' })
);
expect(onSort).toHaveBeenNthCalledWith(1, 'title', 'asc', 'push');
expect(onSort).toHaveBeenNthCalledWith(1, 'title', 'asc', 'push', false);

updatedProps = {
...updatedProps,
Expand All @@ -240,7 +240,7 @@ describe('Card View', () => {
name: 'Sort by TITLE, current direction ascending',
})
);
expect(onSort).toHaveBeenNthCalledWith(2, 'title', 'desc', 'push');
expect(onSort).toHaveBeenNthCalledWith(2, 'title', 'desc', 'push', false);

updatedProps = {
...updatedProps,
Expand All @@ -253,7 +253,7 @@ describe('Card View', () => {
name: 'Sort by TITLE, current direction descending',
})
);
expect(onSort).toHaveBeenNthCalledWith(3, 'title', null, 'push');
expect(onSort).toHaveBeenNthCalledWith(3, 'title', null, 'push', false);
});

it('default sort applied correctly', () => {
Expand All @@ -272,9 +272,9 @@ describe('Card View', () => {
};
render(<CardView {...updatedProps} />);

expect(onSort).toHaveBeenCalledWith('title', 'asc', 'replace');
expect(onSort).toHaveBeenCalledWith('name', 'desc', 'replace');
expect(onSort).toHaveBeenCalledWith('test', 'asc', 'replace');
expect(onSort).toHaveBeenCalledWith('title', 'asc', 'replace', false);
expect(onSort).toHaveBeenCalledWith('name', 'desc', 'replace', false);
expect(onSort).toHaveBeenCalledWith('test', 'asc', 'replace', false);
});

it('can sort by description with label', async () => {
Expand All @@ -290,7 +290,7 @@ describe('Card View', () => {
await user.click(
await screen.findByRole('button', { name: 'Sort by NAME' })
);
expect(onSort).toHaveBeenCalledWith('name', 'asc', 'push');
expect(onSort).toHaveBeenCalledWith('name', 'asc', 'push', false);
});

it('can sort by description without label', async () => {
Expand All @@ -306,7 +306,7 @@ describe('Card View', () => {
await user.click(
await screen.findByRole('button', { name: 'Sort by NAME' })
);
expect(onSort).toHaveBeenCalledWith('name', 'asc', 'push');
expect(onSort).toHaveBeenCalledWith('name', 'asc', 'push', false);
});

it('page changed when sort applied', async () => {
Expand All @@ -318,7 +318,7 @@ describe('Card View', () => {
await screen.findByRole('button', { name: 'Sort by TITLE' })
);

expect(onSort).toHaveBeenCalledWith('title', 'asc', 'push');
expect(onSort).toHaveBeenCalledWith('title', 'asc', 'push', false);
expect(onPageChange).toHaveBeenCalledWith(1);
});

Expand Down Expand Up @@ -377,7 +377,53 @@ describe('Card View', () => {
await user.click(
await screen.findByRole('button', { name: 'Sort by VISITID' })
);
expect(onSort).toHaveBeenCalledWith('visitId', 'asc', 'push');
expect(onSort).toHaveBeenCalledWith('visitId', 'asc', 'push', false);
});

it('calls onSort with correct parameters when shift key is pressed', async () => {
const updatedProps = {
...props,
title: { dataKey: 'title', disableSort: true },
description: { dataKey: 'name', disableSort: true },
information: [
{ dataKey: 'visitId' },
{
dataKey: 'name',
label: 'Name',
content: (entity: Entity) => entity.name,
},
],
page: 1,
};
render(<CardView {...updatedProps} />);

// Click with shift to sort ascending
await user.keyboard('{Shift>}');
await user.click(
await screen.findByRole('button', { name: 'Sort by VISITID' })
);
await user.keyboard('{/Shift}');

expect(onSort).toHaveBeenCalledWith('visitId', 'asc', 'push', true);

await user.click(
await screen.findByRole('button', { name: 'Sort by NAME' })
);

expect(onSort).toHaveBeenCalledWith('name', 'asc', 'push', false);
});

it('displays correct icon when no sort applied', async () => {
render(<CardView {...props} />);

const cards = await screen.findAllByRole('button', { name: /Sort by/ });
expect(
within(cards[0]).getByTestId(`ArrowDownwardIcon`)
).toBeInTheDocument();

await user.keyboard('{Shift>}');
expect(within(cards[0]).getByTestId(`AddIcon`)).toBeInTheDocument();
await user.keyboard('{/Shift}');
});

it('information displays with content that has no tooltip', async () => {
Expand Down
62 changes: 48 additions & 14 deletions packages/datagateway-common/src/card/cardView.component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import React from 'react';
import { useTranslation } from 'react-i18next';
import AdvancedFilter from './advancedFilter.component';
import EntityCard, { EntityImageDetails } from './entityCard.component';
import AddIcon from '@mui/icons-material/Add';

const SelectedChips = styled('ul')(({ theme }) => ({
display: 'inline-flex',
Expand Down Expand Up @@ -85,7 +86,8 @@ export interface CardViewProps {
onSort: (
sort: string,
order: Order | null,
updateMethod: UpdateMethod
updateMethod: UpdateMethod,
shiftDown?: boolean
) => void;

// Props to get title, description of the card
Expand Down Expand Up @@ -189,6 +191,30 @@ const CardView = (props: CardViewProps): React.ReactElement => {
sort,
} = props;

const [shiftDown, setShiftDown] = React.useState(false);
// add event listener to listen for shift key being pressed
React.useEffect(() => {
const handleKeyDown = (event: KeyboardEvent): void => {
if (event.key === 'Shift') {
setShiftDown(true);
}
};

const handleKeyUp = (event: KeyboardEvent): void => {
if (event.key === 'Shift') {
setShiftDown(false);
}
};

document.addEventListener('keydown', handleKeyDown);
document.addEventListener('keyup', handleKeyUp);

return (): void => {
document.removeEventListener('keydown', handleKeyDown);
document.removeEventListener('keyup', handleKeyUp);
};
}, []);

// Results options (by default it is 10, 20 and 30).
const resOptions = React.useMemo(
() =>
Expand Down Expand Up @@ -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);
if (
description &&
description.defaultSort !== undefined &&
sort[description.dataKey] === undefined
)
onSort(description.dataKey, description.defaultSort, 'replace');
onSort(description.dataKey, description.defaultSort, 'replace', false);
if (information) {
information.forEach((element: CardViewDetails) => {
if (
element.defaultSort !== undefined &&
sort[element.dataKey] === undefined
)
onSort(element.dataKey, element.defaultSort, 'replace');
onSort(element.dataKey, element.defaultSort, 'replace', false);
});
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -593,11 +619,12 @@ const CardView = (props: CardViewProps): React.ReactElement => {
<ListItem
key={i}
button
onClick={() => {
onClick={(event) => {
onSort(
s.dataKey,
nextSortDirection(s.dataKey),
'push'
'push',
event.shiftKey
);
if (page !== 1) {
onPageChange(1);
Expand All @@ -615,14 +642,21 @@ const CardView = (props: CardViewProps): React.ReactElement => {
>
<ListItemText primary={s.label} />
<ListItemIcon>
<TableSortLabel
active={s.dataKey in sort}
direction={sort[s.dataKey]}
// Set tabindex to -1 to prevent button focus
tabIndex={-1}
>
{s.dataKey in sort && sort[s.dataKey]}
</TableSortLabel>
{
<TableSortLabel
active={s.dataKey in sort || shiftDown}
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>
}
</ListItemIcon>
</ListItem>
))}
Expand Down
20 changes: 12 additions & 8 deletions ...gateway-common/src/table/headerRenderers/__snapshots__/dataHeader.component.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ exports[`Data column header component renders correctly with sort and filter 1`]
class="MuiBox-root css-0"
>
<span
class="MuiButtonBase-root MuiTableSortLabel-root tour-dataview-sort css-1qgma8u-MuiButtonBase-root-MuiTableSortLabel-root"
aria-label="Test"
class="MuiButtonBase-root MuiTableSortLabel-root Mui-active tour-dataview-sort css-f4w4o6-MuiButtonBase-root-MuiTableSortLabel-root"
data-mui-internal-clone-element="true"
role="button"
tabindex="0"
>
Expand All @@ -142,13 +144,13 @@ exports[`Data column header component renders correctly with sort and filter 1`]
</p>
<svg
aria-hidden="true"
class="MuiSvgIcon-root MuiSvgIcon-fontSizeMedium MuiTableSortLabel-icon MuiTableSortLabel-iconDirectionAsc css-1vweko9-MuiSvgIcon-root-MuiTableSortLabel-icon"
data-testid="ArrowDownwardIcon"
class="MuiSvgIcon-root MuiSvgIcon-fontSizeMedium MuiTableSortLabel-icon MuiTableSortLabel-iconDirectionDesc css-3l415a-MuiSvgIcon-root-MuiTableSortLabel-icon"
data-testid="SortIcon"
focusable="false"
viewBox="0 0 24 24"
>
<path
d="M20 12l-1.41-1.41L13 16.17V4h-2v12.17l-5.58-5.59L4 12l8 8 8-8z"
d="M3 18h6v-2H3v2zM3 6v2h18V6H3zm0 7h12v-2H3v2z"
/>
</svg>
</span>
Expand Down Expand Up @@ -257,7 +259,9 @@ exports[`Data column header component renders correctly with sort but no filter
class="MuiBox-root css-0"
>
<span
class="MuiButtonBase-root MuiTableSortLabel-root tour-dataview-sort css-1qgma8u-MuiButtonBase-root-MuiTableSortLabel-root"
aria-label="Test"
class="MuiButtonBase-root MuiTableSortLabel-root Mui-active tour-dataview-sort css-f4w4o6-MuiButtonBase-root-MuiTableSortLabel-root"
data-mui-internal-clone-element="true"
role="button"
tabindex="0"
>
Expand All @@ -268,13 +272,13 @@ exports[`Data column header component renders correctly with sort but no filter
</p>
<svg
aria-hidden="true"
class="MuiSvgIcon-root MuiSvgIcon-fontSizeMedium MuiTableSortLabel-icon MuiTableSortLabel-iconDirectionAsc css-1vweko9-MuiSvgIcon-root-MuiTableSortLabel-icon"
data-testid="ArrowDownwardIcon"
class="MuiSvgIcon-root MuiSvgIcon-fontSizeMedium MuiTableSortLabel-icon MuiTableSortLabel-iconDirectionDesc css-3l415a-MuiSvgIcon-root-MuiTableSortLabel-icon"
data-testid="SortIcon"
focusable="false"
viewBox="0 0 24 24"
>
<path
d="M20 12l-1.41-1.41L13 16.17V4h-2v12.17l-5.58-5.59L4 12l8 8 8-8z"
d="M3 18h6v-2H3v2zM3 6v2h18V6H3zm0 7h12v-2H3v2z"
/>
</svg>
</span>
Expand Down
Loading

0 comments on commit 6923aca

Please sign in to comment.