LookoutV2 table should not overflow beyond the viewport (#2977)

Signed-off-by: Vasilis Themelis <vdthemelis@gmail.com> Co-authored-by: Chris Martin <council_tax@hotmail.com>
armadaproject · Sep 20, 2023 · 304d456 · 304d456
1 parent 0ad68d6
commit 304d456
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 96 deletions.
diff --git a/internal/lookout/ui/README.md b/internal/lookout/ui/README.md
@@ -6,6 +6,12 @@ This project was bootstrapped with [Create React App](https://github.com/faceboo
 
 In the project directory, you can run:
 
+### `yarn openapi`
+
+This step requires Docker.
+
+Generate the OpenAPI client code from the OpenAPI specification. This step is required to be run before the first time the application is run, and any time the OpenAPI specification is updated.
+
 ### `yarn start`
 
 Runs the app in the development mode.\

diff --git a/internal/lookout/ui/src/App.css b/internal/lookout/ui/src/App.css
@@ -6,6 +6,7 @@
 
 .app-content {
   height: 100%;
+  min-height: 0;
   display: flex;
   flex-direction: column;
 }
diff --git a/internal/lookout/ui/src/components/lookoutV2/JobsTableCell.tsx b/internal/lookout/ui/src/components/lookoutV2/JobsTableCell.tsx
@@ -11,18 +11,23 @@ import { matchForColumn } from "../../utils/jobsTableUtils"
 import styles from "./JobsTableCell.module.css"
 import { JobsTableFilter } from "./JobsTableFilter"
 
+
 const sharedCellStyle = {
   padding: 0,
-  "&:hover": {
-    opacity: 0.85,
-  },
   overflowWrap: "normal",
   textOverflow: "ellipsis",
   whiteSpace: "nowrap",
   overflow: "hidden",
   borderRight: "1px solid #cccccc",
 }
 
+const sharedCellStyleWithOpacity = {
+  ...sharedCellStyle,
+  "&:hover": {
+    opacity: 0.85,
+  },
+}
+
 export interface HeaderCellProps {
   header: Header<JobRow, unknown>
   columnResizeMode: ColumnResizeMode
@@ -70,6 +75,7 @@ export function HeaderCell({
           textOverflow: "ellipsis",
           whiteSpace: "nowrap",
           overflow: "hidden",
+          backgroundColor: "#f2f2f2",
         }}
         className={styles.headerCell}
       />
@@ -100,6 +106,7 @@ export function HeaderCell({
           justifyContent: "space-between",
           alignItems: "center",
           margin: 0,
+          backgroundColor: "#f2f2f2",
         }}
       >
         <div
@@ -203,7 +210,7 @@ export const BodyCell = ({ cell, rowIsGroup, rowIsExpanded, onExpandedChange, on
       key={cell.id}
       align={isRightAligned ? "right" : "left"}
       sx={{
-        ...sharedCellStyle,
+        ...sharedCellStyleWithOpacity,
         padding: "2px 8px 2px 8px",
       }}
     >

diff --git a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.test.tsx
@@ -401,7 +401,7 @@ describe("JobsTableContainer", () => {
       await waitFor(() => {
         const rows = getAllByRole("row")
         // Order should be reversed now
-        expect(rows[rows.length - 2]).toHaveTextContent(sorted[0].jobId)
+        expect(rows[rows.length - 1]).toHaveTextContent(sorted[0].jobId)
       })
     })
   })
@@ -561,7 +561,7 @@ describe("JobsTableContainer", () => {
       async () => {
         const table = await screen.findByRole("table", { name: "Jobs table" })
         const rows = await within(table).findAllByRole("row")
-        expect(rows.length).toBe(nDataRows + 2) // One row per data row, plus the header and footer rows
+        expect(rows.length).toBe(nDataRows + 1) // One row per data row, plus the header
       },
       { timeout: 3000 },
     )

diff --git a/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx b/internal/lookout/ui/src/containers/lookoutV2/JobsTableContainer.tsx
@@ -9,7 +9,6 @@ import {
   TableBody,
   TableCell,
   TableContainer,
-  TableFooter,
   TableHead,
   TablePagination,
   TableRow,
@@ -668,95 +667,96 @@ export const JobsTableContainer = ({
     columnsForSelect = columnsForSelect.filter((col) => col.id !== StandardColumnId.Count)
   }
   return (
-    <Box sx={{ display: "flex", width: "100%" }}>
-      <Box sx={{ overflowX: "auto", overflowY: "auto", margin: "0.5em", width: "100%" }}>
-        <JobsTableActionBar
-          isLoading={rowsToFetch.length > 0}
-          allColumns={columnsForSelect}
-          groupedColumns={grouping}
-          visibleColumns={visibleColumnIds}
-          selectedItemFilters={selectedItemsFilters}
-          customViews={customViews}
-          activeJobSets={activeJobSets}
-          onActiveJobSetsChanged={(newVal) => {
-            setActiveJobSets(newVal)
-            onRefresh()
-          }}
-          onRefresh={onRefresh}
-          onAddAnnotationColumn={addAnnotationCol}
-          onRemoveAnnotationColumn={removeAnnotationCol}
-          onEditAnnotationColumn={editAnnotationCol}
-          onGroupsChanged={onGroupingChange}
-          toggleColumnVisibility={onColumnVisibilityChange}
-          getJobsService={getJobsService}
-          updateJobsService={updateJobsService}
-          onClearFilters={clearFilters}
-          onAddCustomView={addCustomView}
-          onDeleteCustomView={deleteCustomView}
-          onLoadCustomView={loadCustomView}
-        />
-        <TableContainer component={Paper}>
-          <Table
-            sx={{ tableLayout: "fixed" }}
-            aria-label="Jobs table"
-            style={{
-              width: table.getCenterTotalSize(),
-              borderLeft: "1px solid #cccccc",
+    <Box sx={{ display: "flex", flexDirection: "row", height: "100%", width: "100%" }}>
+      <Box sx={{ display: "flex", flexDirection: "column", marginX: "0.5em", height: "100%", width: "100%", minWidth: 0, flex: 1 }}>
+        <Box sx={{ display: "flex", flexDirection: "column", marginY: "0.5em", height: "100%", minHeight: 0, width: "100%", flex: 1 }}>
+          <JobsTableActionBar
+            isLoading={rowsToFetch.length > 0}
+            allColumns={columnsForSelect}
+            groupedColumns={grouping}
+            visibleColumns={visibleColumnIds}
+            selectedItemFilters={selectedItemsFilters}
+            customViews={customViews}
+            activeJobSets={activeJobSets}
+            onActiveJobSetsChanged={(newVal) => {
+              setActiveJobSets(newVal)
+              onRefresh()
             }}
-          >
-            <TableHead>
-              {table.getHeaderGroups().map((headerGroup) => (
-                <TableRow key={headerGroup.id}>
-                  {headerGroup.headers.map((header) => (
-                    <HeaderCell
-                      header={header}
-                      key={header.id}
-                      parseError={header.id in parseErrors ? parseErrors[header.id] : undefined}
-                      columnResizeMode={columnResizeMode}
-                      deltaOffset={table.getState().columnSizingInfo.deltaOffset ?? 0}
-                      columnMatches={columnMatches}
-                      onColumnMatchChange={onColumnMatchChange}
-                      onSetTextFieldRef={(ref) => {
-                        setTextFieldRef(header.id, ref)
-                      }}
-                    />
-                  ))}
-                </TableRow>
-              ))}
-            </TableHead>
-
-            <JobsTableBody
-              dataIsLoading={rowsToFetch.length > 0}
-              columns={table.getVisibleLeafColumns()}
-              topLevelRows={topLevelRows}
-              sidebarJobId={sidebarJobId}
-              onLoadMoreSubRows={onLoadMoreSubRows}
-              onClickRowCheckbox={(row) => selectRow(row, false)}
-              onClickJobRow={toggleSidebarForJobRow}
-              onClickRow={(row) => selectRow(row, true)}
-              onShiftClickRow={shiftSelectRow}
-              onControlClickRow={(row) => selectRow(row, false)}
-            />
-
-            <TableFooter>
-              <TableRow>
-                <TablePagination
-                  rowsPerPageOptions={PAGE_SIZE_OPTIONS}
-                  count={totalRowCount}
-                  rowsPerPage={pageSize}
-                  page={pageIndex}
-                  onPageChange={(_, page) => table.setPageIndex(page)}
-                  onRowsPerPageChange={(e) => table.setPageSize(Number(e.target.value))}
-                  colSpan={table.getVisibleLeafColumns().length}
-                  showFirstButton={true}
-                  showLastButton={true}
-                />
-              </TableRow>
-            </TableFooter>
-          </Table>
-        </TableContainer>
-
-        {debug && <pre>{JSON.stringify(table.getState(), null, 2)}</pre>}
+            onRefresh={onRefresh}
+            onAddAnnotationColumn={addAnnotationCol}
+            onRemoveAnnotationColumn={removeAnnotationCol}
+            onEditAnnotationColumn={editAnnotationCol}
+            onGroupsChanged={onGroupingChange}
+            toggleColumnVisibility={onColumnVisibilityChange}
+            getJobsService={getJobsService}
+            updateJobsService={updateJobsService}
+            onClearFilters={clearFilters}
+            onAddCustomView={addCustomView}
+            onDeleteCustomView={deleteCustomView}
+            onLoadCustomView={loadCustomView}
+          />
+          <TableContainer
+            component={Paper}
+            style={{ height: "100%" }}>
+            <Table stickyHeader
+              sx={{ tableLayout: "fixed" }}
+              aria-label="Jobs table"
+              style={{
+                width: table.getCenterTotalSize(),
+                borderLeft: "1px solid #cccccc",
+                height: "100%",
+              }}
+            >
+              <TableHead>
+                {table.getHeaderGroups().map((headerGroup) => (
+                  <TableRow key={headerGroup.id}>
+                    {headerGroup.headers.map((header) => (
+                      <HeaderCell
+                        header={header}
+                        key={header.id}
+                        parseError={header.id in parseErrors ? parseErrors[header.id] : undefined}
+                        columnResizeMode={columnResizeMode}
+                        deltaOffset={table.getState().columnSizingInfo.deltaOffset ?? 0}
+                        columnMatches={columnMatches}
+                        onColumnMatchChange={onColumnMatchChange}
+                        onSetTextFieldRef={(ref) => {
+                          setTextFieldRef(header.id, ref)
+                        }}
+                      />
+                    ))}
+                  </TableRow>
+                ))}
+              </TableHead>
+
+              <JobsTableBody
+                dataIsLoading={rowsToFetch.length > 0}
+                columns={table.getVisibleLeafColumns()}
+                topLevelRows={topLevelRows}
+                sidebarJobId={sidebarJobId}
+                onLoadMoreSubRows={onLoadMoreSubRows}
+                onClickRowCheckbox={(row) => selectRow(row, false)}
+                onClickJobRow={toggleSidebarForJobRow}
+                onClickRow={(row) => selectRow(row, true)}
+                onShiftClickRow={shiftSelectRow}
+                onControlClickRow={(row) => selectRow(row, false)}
+              />
+            </Table>
+          </TableContainer>
+          <TablePagination
+            component="div"
+            rowsPerPageOptions={PAGE_SIZE_OPTIONS}
+            count={totalRowCount}
+            rowsPerPage={pageSize}
+            page={pageIndex}
+            onPageChange={(_, page) => table.setPageIndex(page)}
+            onRowsPerPageChange={(e) => table.setPageSize(Number(e.target.value))}
+            colSpan={table.getVisibleLeafColumns().length}
+            showFirstButton={true}
+            showLastButton={true}
+          />
+
+          {debug && <pre>{JSON.stringify(table.getState(), null, 2)}</pre>}
+        </Box>
       </Box>
 
       {sidebarJobDetails !== undefined && (
@@ -804,7 +804,9 @@ const JobsTableBody = ({
 }: JobsTableBodyProps) => {
   const canDisplay = !dataIsLoading && topLevelRows.length > 0
   return (
-    <TableBody>
+    <TableBody
+      style= {{ overflow: "auto", height: "100%" }}
+    >
       {!canDisplay && (
         <TableRow>
           {dataIsLoading && topLevelRows.length === 0 && (