-
Notifications
You must be signed in to change notification settings - Fork 92
feat(new-webui): Add keyboard scroll support to AntD virtual table used by search results and ingest jobs (resolves #944). #972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1a58919
b869ab5
cd9b3c4
603a806
79da1b5
1c36d17
b97724f
c64dbec
1647193
2859dea
2d4bb10
815c10b
a961690
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import React, { | ||
| useCallback, | ||
| useRef, | ||
| } from "react"; | ||
|
|
||
| import {Table} from "antd"; | ||
|
|
||
| import { | ||
| SCROLL_INCREMENT, | ||
| VIRTUAL_TABLE_HOLDER_SELECTOR, | ||
| type VirtualTableProps, | ||
| } from "./typings"; | ||
|
|
||
|
|
||
| /** | ||
| * Virtual table that supports keyboard navigation. | ||
| * | ||
| * @param props | ||
| * @param props.tableProps | ||
| * @return | ||
| */ | ||
| const VirtualTable = <RecordType extends object = Record<string, unknown>>({ | ||
| ...tableProps | ||
| }: VirtualTableProps<RecordType>) => { | ||
| const containerRef = useRef<HTMLDivElement>(null); | ||
| const scrollNodeRef = useRef<HTMLElement>(null); | ||
|
|
||
| const handleKeyDown = useCallback((e: React.KeyboardEvent<HTMLDivElement>) => { | ||
| if (null === containerRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| const scrollNode = scrollNodeRef.current; | ||
| if (null === scrollNode) { | ||
| scrollNodeRef.current = containerRef.current.querySelector<HTMLElement>( | ||
| VIRTUAL_TABLE_HOLDER_SELECTOR | ||
| ); | ||
| } | ||
|
|
||
| if (null === scrollNode) { | ||
| return; | ||
| } | ||
|
|
||
| const visibleTableHeight = scrollNode.clientHeight; | ||
| let {scrollTop} = scrollNode; | ||
|
|
||
| switch (e.key) { | ||
| case "ArrowDown": | ||
| scrollTop += SCROLL_INCREMENT; | ||
| break; | ||
| case "ArrowUp": | ||
| // Prevent scrolling past the top. | ||
| scrollTop = Math.max(scrollTop - SCROLL_INCREMENT, 0); | ||
| break; | ||
| case "PageDown": | ||
| scrollTop += visibleTableHeight; | ||
| break; | ||
| case "PageUp": | ||
| // Prevent scrolling past the top. | ||
| scrollTop = Math.max(scrollTop - visibleTableHeight, 0); | ||
| break; | ||
| case "Home": | ||
| scrollTop = 0; | ||
| break; | ||
| case "End": | ||
| // Scroll to the bottom of the table. | ||
| scrollTop = Number.MAX_SAFE_INTEGER; | ||
| break; | ||
| default: | ||
| return; | ||
| } | ||
|
|
||
| scrollNode.scrollTop = scrollTop; | ||
| e.preventDefault(); | ||
| }, []); | ||
|
|
||
| return ( | ||
| <div | ||
| ref={containerRef} | ||
| style={{outline: "none"}} | ||
| tabIndex={0} | ||
| onKeyDown={handleKeyDown} | ||
| > | ||
| <Table<RecordType> | ||
| virtual={true} | ||
| {...tableProps}/> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default VirtualTable; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import type {TableProps} from "antd"; | ||
|
|
||
|
|
||
| /** | ||
| * Number of pixels to scroll vertically when using keyboard arrow navigation. | ||
| */ | ||
| const SCROLL_INCREMENT = 32; | ||
|
|
||
| /** | ||
| * CSS selector for the virtual table body element. | ||
| */ | ||
| const VIRTUAL_TABLE_HOLDER_SELECTOR = ".ant-table-tbody-virtual-holder"; | ||
|
|
||
| /** | ||
| * Antd Table props with virtual omitted since set by VirtualTable. | ||
| */ | ||
| type VirtualTableProps<RecordType> = Omit<TableProps<RecordType>, "virtual">; | ||
|
|
||
| export { | ||
| SCROLL_INCREMENT, | ||
| VIRTUAL_TABLE_HOLDER_SELECTOR, | ||
| }; | ||
| export type {VirtualTableProps}; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,10 +4,10 @@ import { | |||||
| useState, | ||||||
| } from "react"; | ||||||
|
|
||||||
| import {Table} from "antd"; | ||||||
| import dayjs from "dayjs"; | ||||||
|
|
||||||
| import {DashboardCard} from "../../../components/DashboardCard"; | ||||||
| import VirtualTable from "../../../components/VirtualTable"; | ||||||
| import useIngestStatsStore from "../ingestStatsStore"; | ||||||
| import {querySql} from "../sqlConfig"; | ||||||
| import styles from "./index.module.css"; | ||||||
|
|
@@ -85,11 +85,12 @@ const Jobs = ({className}: JobsProps) => { | |||||
| return ( | ||||||
| <div className={className}> | ||||||
| <DashboardCard title={"Ingestion Jobs"}> | ||||||
| <Table<JobData> | ||||||
| <VirtualTable<JobData> | ||||||
| className={styles["jobs"] || ""} | ||||||
| columns={jobColumns} | ||||||
| dataSource={jobs} | ||||||
| pagination={false}/> | ||||||
| pagination={false} | ||||||
| scroll={{y: 400}}/> | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider making the fixed scroll height responsive. The 400px fixed height might not work well on smaller screens or different viewport sizes. Consider calculating height dynamically like in SearchResultsTable. - scroll={{y: 400}}/>
+ scroll={{y: Math.min(400, window.innerHeight * 0.6)}}/>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| </DashboardCard> | ||||||
| </div> | ||||||
| ); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,7 @@ import { | |
| useState, | ||
| } from "react"; | ||
|
|
||
| import {Table} from "antd"; | ||
|
|
||
| import VirtualTable from "../../../../components/VirtualTable"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Deep relative import path – consider aliasing. The import for 🤖 Prompt for AI Agents |
||
| import useSearchStore from "../../SearchState/index"; | ||
| import { | ||
| SearchResult, | ||
|
|
@@ -57,16 +56,16 @@ const SearchResultsTable = () => { | |
| }, []); | ||
|
|
||
| return ( | ||
| <div ref={containerRef}> | ||
| <Table<SearchResult> | ||
| <div | ||
| ref={containerRef} | ||
| style={{outline: "none"}} | ||
| > | ||
| <VirtualTable<SearchResult> | ||
| columns={searchResultsTableColumns} | ||
| dataSource={searchResults || []} | ||
| pagination={false} | ||
| rowKey={(record) => record._id.toString()} | ||
| scroll={{y: tableHeight}} | ||
| virtual={true} | ||
| dataSource={searchResults ? | ||
| searchResults : | ||
| []}/> | ||
| scroll={{y: tableHeight}}/> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use scrollHeight instead of MAX_SAFE_INTEGER for "End" key.
Setting scrollTop to MAX_SAFE_INTEGER may not reliably scroll to the bottom. Use the element's scrollHeight for more predictable behaviour.
case "End": // Scroll to the bottom of the table. - scrollTop = Number.MAX_SAFE_INTEGER; + scrollTop = scrollNode.scrollHeight - scrollNode.clientHeight; break;📝 Committable suggestion
🤖 Prompt for AI Agents