diff --git a/Changelog.md b/Changelog.md index f842ff7d43..66401f94a6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ ### 🚨 Breaking changes ### ✨ New features and improvements +- Added row numbers to tables using React Table v8, allowing users to identify row positions after sorting and filtering (#8089) - Allowed instructors assigned as graders to switch between all submissions and only their assigned submissions in the submissions, summary, and grading views (#8083) - Improved Session Timeout Logic: `check_timeout` polling paused when user is not focused on the MarkUs tab and polling stops after user session has timed out (#8074) - Migrated Groups Manager students and groups tables to use `react-table` v8 (#8068) diff --git a/app/assets/stylesheets/common/_constants.scss b/app/assets/stylesheets/common/_constants.scss index 2692d16d68..a07d491576 100644 --- a/app/assets/stylesheets/common/_constants.scss +++ b/app/assets/stylesheets/common/_constants.scss @@ -18,6 +18,7 @@ --primary_two: #cee3ea; --primary_three: #89b1dd; --radius: 5px; + --row-number-gutter-width: 30px; --severe_alert: #ffd452; --severe_error: #a20000; --severe_success: #246700; diff --git a/app/assets/stylesheets/common/_table.scss b/app/assets/stylesheets/common/_table.scss index d5624d48c8..6f7fee4a2b 100644 --- a/app/assets/stylesheets/common/_table.scss +++ b/app/assets/stylesheets/common/_table.scss @@ -72,6 +72,7 @@ } .rt-tr { + background-color: $background-support; text-align: center; } @@ -210,6 +211,36 @@ .rt-tr { flex: 1 0 auto; display: inline-flex; + position: relative; + } + + &.-show-row-numbers { + .rt-tbody { + counter-reset: table-row-number; + + .rt-tr { + counter-increment: table-row-number; + + &::before { + align-items: center; + background-color: $background-support; + bottom: 0; + content: counter(table-row-number); + display: flex; + font-size: 0.85em; + font-variant-numeric: tabular-nums; + justify-content: center; + left: 0; + position: absolute; + top: 0; + width: var(--row-number-gutter-width); + } + } + } + + .rt-tr { + padding-left: var(--row-number-gutter-width); + } } .rt-th, diff --git a/app/javascript/Components/__tests__/assignment_summary.test.jsx b/app/javascript/Components/__tests__/assignment_summary.test.jsx index b8bc157850..b2c1be2c52 100644 --- a/app/javascript/Components/__tests__/assignment_summary.test.jsx +++ b/app/javascript/Components/__tests__/assignment_summary.test.jsx @@ -82,6 +82,10 @@ describe("For the AssignmentSummaryTable's display of inactive groups", () => { ); }); + it("shows row numbers", () => { + expect(document.querySelector(".Table")).toHaveClass("-show-row-numbers"); + }); + it("initially does not contain the inactive group", () => { expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument(); }); diff --git a/app/javascript/Components/__tests__/instructor_table.test.jsx b/app/javascript/Components/__tests__/instructor_table.test.jsx index e72f7cb177..602d924eee 100644 --- a/app/javascript/Components/__tests__/instructor_table.test.jsx +++ b/app/javascript/Components/__tests__/instructor_table.test.jsx @@ -20,7 +20,7 @@ describe("For the InstructorTable's display of instructors", () => { const instructors_in_one_row = instructor => { const rows = screen.getAllByRole("row"); for (let row of rows) { - const cells = Array.from(row.childNodes).map(c => c.textContent); + const cells = Array.from(row.querySelectorAll(".rt-td"), c => c.textContent); if (cells[0] === instructor.user_name) { expect(cells[1]).toEqual(instructor.first_name); expect(cells[2]).toEqual(instructor.last_name); diff --git a/app/javascript/Components/__tests__/student_table.test.jsx b/app/javascript/Components/__tests__/student_table.test.jsx index cf6948c65b..24e35e1dd0 100644 --- a/app/javascript/Components/__tests__/student_table.test.jsx +++ b/app/javascript/Components/__tests__/student_table.test.jsx @@ -119,7 +119,7 @@ describe("For the StudentTable's display of students", () => { const student_in_one_row = student => { const rows = screen.getAllByRole("row"); for (let row of rows) { - const cells = Array.from(row.childNodes).map(c => c.textContent); + const cells = Array.from(row.querySelectorAll(".rt-td"), c => c.textContent); if (cells[1] === student.user_name) { expect(cells[2]).toEqual(student.first_name); expect(cells[3]).toEqual(student.last_name); diff --git a/app/javascript/Components/__tests__/table.test.jsx b/app/javascript/Components/__tests__/table.test.jsx index 1bdb74f656..c39368bf4b 100644 --- a/app/javascript/Components/__tests__/table.test.jsx +++ b/app/javascript/Components/__tests__/table.test.jsx @@ -177,6 +177,30 @@ describe("tests for the table component", () => { expectRowsInTableInOrder(table, columns, data); }); + + it("does not show row numbers by default", () => { + const {table} = renderTableWithMockData(); + const tableElement = table.querySelector(".Table"); + + expect(tableElement).not.toHaveClass("-show-row-numbers"); + }); + + it("shows positional row numbers without adding a table column when enabled", () => { + const {table} = renderTableWithMockData({showRowNumbers: true}); + const tableElement = table.querySelector(".Table"); + const header = table.querySelector(".rt-thead.-header"); + const tableRows = table.querySelectorAll(".rt-tbody .rt-tr"); + + expect(tableElement).toHaveClass("-show-row-numbers"); + expect(table.querySelector(".rt-tbody").style.minWidth).toContain( + "var(--row-number-gutter-width)" + ); + expect(header.querySelectorAll(".rt-th")).toHaveLength(mockColumns().length); + tableRows.forEach(tableRow => { + expect(tableRow.querySelectorAll(".rt-td")).toHaveLength(mockColumns().length); + expect(tableRow.querySelector(".rt-row-number")).not.toBeInTheDocument(); + }); + }); }); describe("rendering of the no-data component when data is empty", () => { @@ -226,6 +250,19 @@ describe("tests for the table component", () => { expectRowsInTableInOrder(table, columns, data); } }); + + it("sorts row contents without adding a row-number column", async () => { + const {table, columns, data} = renderTableWithMockData({showRowNumbers: true}); + + await clickHeader(columns[0].header); + await clickHeader(columns[0].header); + + const tableRows = table.querySelectorAll(".rt-tbody .rt-tr"); + expect(within(tableRows[0]).getByText(data[data.length - 1].col1)).toBeInTheDocument(); + tableRows.forEach(tableRow => + expect(tableRow.querySelectorAll(".rt-td")).toHaveLength(columns.length) + ); + }); }); describe("filtering", () => { @@ -246,7 +283,7 @@ describe("tests for the table component", () => { }); it("filters data", async () => { - const {columns} = renderTableWithMockData(); + const {table, columns} = renderTableWithMockData(); columns.pop(); const inputs = searchInputs(); @@ -266,6 +303,7 @@ describe("tests for the table component", () => { ], columns ); + expect(table.querySelectorAll(".rt-tbody .rt-tr")).toHaveLength(2); }); it("resets the filter when the search query is erased", async () => { diff --git a/app/javascript/Components/assignment_summary_table.jsx b/app/javascript/Components/assignment_summary_table.jsx index 8e814f44f0..e1be02c341 100644 --- a/app/javascript/Components/assignment_summary_table.jsx +++ b/app/javascript/Components/assignment_summary_table.jsx @@ -541,6 +541,7 @@ export class AssignmentSummaryTable extends React.Component { }); }} loading={this.state.loading} + showRowNumbers /> +
{table.getHeaderGroups().map(headerGroup => (
{headerGroup.headers.map(header => ( @@ -200,7 +205,7 @@ export default function Table({ [table, finalColumns] ); const tableFilters = showFilters && ( -
+
{table.getHeaderGroups().map(headerGroup => (
{headerGroup.headers.map(header => ( @@ -222,12 +227,15 @@ export default function Table({ ); return ( -
+
{tableHeaders} {tableFilters} -
- {table.getRowModel().rows.map(row => ( +
+ {rows.map(row => ( ))} - {loading && table.getRowModel().rows.length > 0 && ( + {loading && rows.length > 0 && (
)} - {!table.getRowModel().rows.length && + {!rows.length && (loading ? (