diff --git a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx index 85d006c401..4169bdcd93 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/AssignmentActivity.tsx @@ -72,12 +72,27 @@ export default function AssignmentActivity() { students.error ? 'Could not load students' : 'No students found' } rows={rows} - columnNames={{ - display_name: 'Student', - annotations: 'Annotations', - replies: 'Replies', - last_activity: 'Last Activity', - }} + columns={[ + { + field: 'display_name', + label: 'Student', + }, + { + field: 'annotations', + label: 'Annotations', + initialOrderDirection: 'descending', + }, + { + field: 'replies', + label: 'Replies', + initialOrderDirection: 'descending', + }, + { + field: 'last_activity', + label: 'Last Activity', + initialOrderDirection: 'descending', + }, + ]} defaultOrderField="display_name" renderItem={(stats, field) => { if (['annotations', 'replies'].includes(field)) { diff --git a/lms/static/scripts/frontend_apps/components/dashboard/CourseActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/CourseActivity.tsx index 7b4b75d948..44b3fd59fa 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/CourseActivity.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/CourseActivity.tsx @@ -69,12 +69,27 @@ export default function CourseActivity() { : 'No assignments found' } rows={rows} - columnNames={{ - title: 'Assignment', - annotations: 'Annotations', - replies: 'Replies', - last_activity: 'Last Activity', - }} + columns={[ + { + field: 'title', + label: 'Assignment', + }, + { + field: 'annotations', + label: 'Annotations', + initialOrderDirection: 'descending', + }, + { + field: 'replies', + label: 'Replies', + initialOrderDirection: 'descending', + }, + { + field: 'last_activity', + label: 'Last Activity', + initialOrderDirection: 'descending', + }, + ]} defaultOrderField="title" renderItem={(stats, field) => { if (['annotations', 'replies'].includes(field)) { diff --git a/lms/static/scripts/frontend_apps/components/dashboard/OrderableActivityTable.tsx b/lms/static/scripts/frontend_apps/components/dashboard/OrderableActivityTable.tsx index d503f3e59c..d47ef149eb 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/OrderableActivityTable.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/OrderableActivityTable.tsx @@ -5,11 +5,17 @@ import type { OrderDirection } from '@hypothesis/frontend-shared/lib/types'; import { useMemo, useState } from 'preact/hooks'; import { useLocation } from 'wouter-preact'; +export type OrderableActivityTableColumn = { + field: keyof T; + label: string; + initialOrderDirection?: OrderDirection; +}; + export type OrderableActivityTableProps = Pick< DataTableProps, 'emptyMessage' | 'rows' | 'renderItem' | 'loading' | 'title' > & { - columnNames: Partial>; + columns: OrderableActivityTableColumn[]; defaultOrderField: keyof T; /** @@ -19,25 +25,6 @@ export type OrderableActivityTableProps = Pick< navigateOnConfirmRow?: (row: T) => string; }; -/** - * List of columns which should start sorted in descending order. - * - * This is because for numeric columns ("annotations" and "replies") users will - * usually want to see the higher values first. - * Similarly, for date columns ("last_activity") users will want to see most - * recent values first. - */ -const descendingOrderColumns: readonly string[] = [ - // Annotation metrics - 'last_activity', - 'annotations', - 'replies', - - // Course metrics - 'last_launched', - 'assignments', -]; - /** * Annotation activity table for dashboard views. Includes built-in support for * sorting columns. @@ -45,7 +32,7 @@ const descendingOrderColumns: readonly string[] = [ export default function OrderableActivityTable({ defaultOrderField, rows, - columnNames, + columns, navigateOnConfirmRow, ...restOfTableProps }: OrderableActivityTableProps) { @@ -54,29 +41,26 @@ export default function OrderableActivityTable({ direction: 'ascending', }); const orderedRows = useOrderedRows(rows, order); - const columns = useMemo( + const dataTableColumns = useMemo( () => - Object.entries(columnNames).map(([field, label], index) => ({ - field: field as keyof T, - label: label as string, + columns.map(({ field, label }, index) => ({ + field, + label, classes: index === 0 ? 'w-[60%]' : undefined, })), - [columnNames], + [columns], ); // Map of column name to initial sort order const orderableColumns = useMemo( () => - (Object.keys(columnNames) as Array).reduce< - Partial> - >((acc, columnName) => { - acc[columnName] = - typeof columnName === 'string' && - descendingOrderColumns.includes(columnName) - ? 'descending' - : 'ascending'; - return acc; - }, {}), - [columnNames], + columns.reduce>>( + (acc, { field, initialOrderDirection = 'ascending' }) => { + acc[field] = initialOrderDirection; + return acc; + }, + {}, + ), + [columns], ); const [, navigate] = useLocation(); @@ -84,11 +68,18 @@ export default function OrderableActivityTable({ + setOrder({ + ...order, + // Every column should start with nulls last, and move them first + // when order direction changes + nullsLast: order.direction === orderableColumns[order.field], + }) + } onConfirmRow={ navigateOnConfirmRow ? row => navigate(navigateOnConfirmRow(row)) diff --git a/lms/static/scripts/frontend_apps/components/dashboard/OrganizationActivity.tsx b/lms/static/scripts/frontend_apps/components/dashboard/OrganizationActivity.tsx index 480136ac44..d353c90551 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/OrganizationActivity.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/OrganizationActivity.tsx @@ -55,11 +55,22 @@ export default function OrganizationActivity({ courses.error ? 'Could not load courses' : 'No courses found' } rows={rows} - columnNames={{ - title: 'Course Title', - assignments: 'Assignments', - last_launched: 'Last launched', - }} + columns={[ + { + field: 'title', + label: 'Course title', + }, + { + field: 'assignments', + label: 'Assignments', + initialOrderDirection: 'descending', + }, + { + field: 'last_launched', + label: 'Last launched', + initialOrderDirection: 'descending', + }, + ]} defaultOrderField="title" renderItem={(stats, field) => { if (field === 'assignments') { diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/OrderableActivityTable-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/OrderableActivityTable-test.js index 05eb1f1651..0310a8ab66 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/test/OrderableActivityTable-test.js +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/OrderableActivityTable-test.js @@ -1,4 +1,5 @@ import { mount } from 'enzyme'; +import sinon from 'sinon'; import OrderableActivityTable, { $imports } from '../OrderableActivityTable'; @@ -43,12 +44,24 @@ describe('OrderableActivityTable', () => { return mount( , @@ -58,6 +71,7 @@ describe('OrderableActivityTable', () => { [ { orderToSet: { field: 'annotations', direction: 'descending' }, + expectedNullsLast: false, expectedStudents: [ { display_name: 'b', @@ -81,6 +95,7 @@ describe('OrderableActivityTable', () => { }, { orderToSet: { field: 'replies', direction: 'ascending' }, + expectedNullsLast: true, expectedStudents: [ { display_name: 'b', @@ -104,6 +119,7 @@ describe('OrderableActivityTable', () => { }, { orderToSet: { field: 'last_activity', direction: 'descending' }, + expectedNullsLast: false, expectedStudents: [ { display_name: 'a', @@ -125,7 +141,7 @@ describe('OrderableActivityTable', () => { }, ], }, - ].forEach(({ orderToSet, expectedStudents }) => { + ].forEach(({ orderToSet, expectedNullsLast, expectedStudents }) => { it('reorders students on order change', () => { const wrapper = createComponent(); const getRows = () => wrapper.find('DataTable').prop('rows'); @@ -162,7 +178,10 @@ describe('OrderableActivityTable', () => { ]); setOrder(orderToSet); - assert.deepEqual(getOrder(), orderToSet); + assert.deepEqual(getOrder(), { + ...orderToSet, + nullsLast: expectedNullsLast, + }); assert.deepEqual(getRows(), expectedStudents); }); }); diff --git a/package.json b/package.json index 08ae2d32a0..cac8b14df3 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "@babel/preset-react": "^7.24.6", "@babel/preset-typescript": "^7.24.6", "@hypothesis/frontend-build": "^3.0.0", - "@hypothesis/frontend-shared": "^7.10.1", + "@hypothesis/frontend-shared": "^7.11.0", "@rollup/plugin-babel": "^6.0.4", "@rollup/plugin-commonjs": "^25.0.8", "@rollup/plugin-node-resolve": "^15.2.3", diff --git a/yarn.lock b/yarn.lock index 195adc77d2..5e1e82e55e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1988,15 +1988,15 @@ __metadata: languageName: node linkType: hard -"@hypothesis/frontend-shared@npm:^7.10.1": - version: 7.10.1 - resolution: "@hypothesis/frontend-shared@npm:7.10.1" +"@hypothesis/frontend-shared@npm:^7.11.0": + version: 7.11.0 + resolution: "@hypothesis/frontend-shared@npm:7.11.0" dependencies: highlight.js: ^11.6.0 wouter-preact: ^3.0.0 peerDependencies: preact: ^10.4.0 - checksum: 758c415d8e68325a3c630122e76bd40895cafbfa00404829549380c873aed6e8fd937c27da5a0632240d547c262425b1374b75e951d498d546aa19c6894daff1 + checksum: 12d10503b2a51a8c5d690f0f892341698465f49d3c760574a838918879a7a0151ddf0994f422515264224f6462a5b1baf1169f4f16ca6f5f9b8d7b4d8065cc4b languageName: node linkType: hard @@ -7193,7 +7193,7 @@ __metadata: "@babel/preset-react": ^7.24.6 "@babel/preset-typescript": ^7.24.6 "@hypothesis/frontend-build": ^3.0.0 - "@hypothesis/frontend-shared": ^7.10.1 + "@hypothesis/frontend-shared": ^7.11.0 "@hypothesis/frontend-testing": ^1.2.2 "@rollup/plugin-babel": ^6.0.4 "@rollup/plugin-commonjs": ^25.0.8