Reviewing files that changed from the base of the PR and between fc7dc03 and 79da1b5.

• components/log-viewer-webui/client/src/components/VirtualTable/index.tsx (1 hunks)
• components/log-viewer-webui/client/src/components/VirtualTable/typings.tsx (1 hunks)
• components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (2 hunks)
• components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (2 hunks)

• GitHub Check: lint-check (macos-latest)
• GitHub Check: lint-check (ubuntu-latest)
• GitHub Check: build (ubuntu-latest)

components/log-viewer-webui/client/src/components/VirtualTable/typings.tsx (2)

17-17: Well-designed type definition.

Good design choice to omit the "virtual" property since the VirtualTable component manages virtualization internally.

---

12-12:

Details

❓ Verification inconclusive

CSS selector dependency on internal antd classes poses maintenance risk.

This selector targets antd's internal class structure which could break with library updates. Consider documenting this dependency and adding fallback logic.

---

🏁 Script executed:

#!/bin/bash
# Description: Check if this CSS selector is documented in antd source or mentioned in other projects
# Expected: Find usage patterns or documentation

rg -A 3 -B 3 "ant-table-tbody-virtual-holder" --type typescript --type javascript

Length of output: 116

---

🏁 Script executed:

#!/bin/bash
# Re-run search for the internal antd CSS class across all JS/TS files without relying on ripgrep’s --type filters
rg -A 3 -B 3 "ant-table-tbody-virtual-holder" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'

Length of output: 825

---

Document and Mitigate Internal Antd CSS Dependency
The selector .ant-table-tbody-virtual-holder only appears here and targets Ant Design’s internal DOM structure for virtual scrolling. Relying on it risks breakage when upgrading antd.

• Document this dependency (e.g. in your component JSDoc or project README)
• Add fallback logic or feature-detection in case the class name changes
• Whenever you bump antd, verify that this selector still applies
• If possible, prefer a public API (e.g. custom row renderers or built-in virtual list components) over internal classes

components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (1)

10-10: Good component replacement for virtual scrolling support.

The switch to VirtualTable enables keyboard navigation for the jobs table, which improves accessibility.

components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (3)

7-7: Good import of the new VirtualTable component.

This enables keyboard scrolling support for search results.

---

44-47: Container styling aligns with VirtualTable focus management.

The outline: "none" style is appropriate since VirtualTable handles focus styling internally.

---

50-50: Explicit null handling improves robustness.

Good defensive programming to explicitly handle the case where searchResults might be null or undefined.

components/log-viewer-webui/client/src/components/VirtualTable/index.tsx (3)

27-30: Proper null check follows coding guidelines.

The use of null === containerRef.current correctly follows the coding guideline preference for explicit equality checks.

---

77-77: TabIndex usage is appropriate despite static analysis warning.

The static analysis tool flagged tabIndex=0 on a div, but this is actually correct for keyboard navigation. The div needs to be focusable to receive keyboard events for table scrolling.

🧰 Tools

🪛 Biome (1.9.4)

[error] 77-77: The HTML element div is non-interactive. Do not use tabIndex.

Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.

(lint/a11y/noNoninteractiveTabindex)

---

81-81:

Details

✅ Verification successful

Verify virtual prop is always effective.

Ensure that virtual={true} always enables virtualization in antd tables, or add fallback handling if virtualization fails to activate.

---

🏁 Script executed:

#!/bin/bash
# Description: Check antd documentation for virtual table requirements
# Expected: Find conditions when virtual mode is enabled

rg -A 10 -B 5 "virtual.*true" --type typescript --type javascript

Length of output: 102

---

🏁 Script executed:

#!/bin/bash
# Inspect virtualization setup in the VirtualTable component

# 1. Locate the VirtualTable file
echo "File path:"
rg -l "virtual={true}" -g "*.tsx"

# 2. Show context around Table component usage
echo -e "\nTable usage in VirtualTable:"
rg -n "Table" -C3 components/log-viewer-webui/client/src/components/VirtualTable/index.tsx

# 3. Show where virtual prop is applied
echo -e "\nvirtual prop context:"
rg -n "virtual" -C3 components/log-viewer-webui/client/src/components/VirtualTable/index.tsx

# 4. Check scroll prop presence (required for antd virtualization)
echo -e "\nscroll prop context:"
rg -n "scroll" -C3 components/log-viewer-webui/client/src/components/VirtualTable/index.tsx

Length of output: 3676

---

Virtual Table virtualization fallback already handled

The virtual={true} prop on Ant Design’s Table always attempts to enable virtualization. In our VirtualTable wrapper we guard against failures by checking for the virtualization container before applying keyboard-driven scrolling:

• In components/log-viewer-webui/client/src/components/VirtualTable/index.tsx around lines 32–38, we do:

const scrollNode = containerRef.current.querySelector<HTMLElement>(
  VIRTUAL_TABLE_HOLDER_SELECTOR
);
if (scrollNode === null) {
  return; // fallback: skip virtualization if container not found
}

This early return ensures that, if virtualization doesn’t activate for any reason, no scroll operations are attempted. No further changes are needed here.

Reviewing files that changed from the base of the PR and between 2d4bb10 and a961690.

• components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (2 hunks)

• GitHub Check: antlr-code-committed (ubuntu-latest)
• GitHub Check: lint-check (ubuntu-latest)

components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (4)

63-63: Good: explicit generic for type safety.

Providing <VirtualTable<SearchResult>> ensures the table’s props and columns align with the SearchResult shape. Strong typing here is beneficial.

---

65-65: Solid fallback for empty data.

Using dataSource={searchResults || []} avoids null or undefined issues when there are no results. No changes needed.

---

68-68:

Details

❓ Verification inconclusive

Avoid initial zero-height flicker.

With tableHeight initialised to 0, the table renders at zero height on first paint, causing a brief flicker. Consider switching to useLayoutEffect for DOM measurements or setting an initial height based on a one-time calculation:

-import {useEffect, useRef, useState} from "react";
+import {useLayoutEffect, useRef, useState} from "react";
...
-    useEffect(() => {
+    useLayoutEffect(() => {
         const updateHeight = () => {
             /* ... */
         };
         /* ... */

Please verify that this change eliminates any layout jumps on mount.

---

🏁 Script executed:

#!/usr/bin/env bash
set -e
FILE="components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx"

echo "----- Searching for useState -----"
rg -C3 "useState" "$FILE" || true

echo "----- Searching for useEffect -----"
rg -C3 "useEffect" "$FILE" || true

echo "----- Searching for useLayoutEffect -----"
rg -C3 "useLayoutEffect" "$FILE" || true

Length of output: 1789

---

Switch DOM measurements to useLayoutEffect to prevent initial flicker

Replace the height-calculation effect so it runs before paint, ensuring tableHeight is set on first render:

• File:
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx

- import {useEffect, useRef, useState} from "react";
+ import {useLayoutEffect, useRef, useState} from "react";
...
-    useEffect(() => {
+    useLayoutEffect(() => {
        const updateHeight = () => {
            if (containerRef.current) {
                const {top} = containerRef.current.getBoundingClientRect();
                setTableHeight(window.innerHeight - top - PADDING);
            }
        };
        window.addEventListener("resize", updateHeight);
        updateHeight();
        return () => window.removeEventListener("resize", updateHeight);
    }, [/* your deps */]);

• Leave other effects (e.g. result-count tracking) on useEffect.

Please verify that this change removes any layout jumps on mount.

---

59-62:

Details

❌ Incorrect review comment

Ensure container is focusable for keyboard events.

The <div> wrapper has outline: "none" but no tabIndex, so it won’t receive focus or keyboard events by default. Add tabIndex={0} (and move outline: none into a focus style) so keyboard navigation actually reaches the container:

-       <div
+       <div
            ref={containerRef}
+           tabIndex={0}
            style={{outline: "none"}}
         >

---

🏁 Script executed:

#!/bin/bash
# Verify if the containerRef is used for keyboard handling in SearchResultsTable
rg -n "containerRef" components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx
rg -n "onKey" components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx
rg -n "addEventListener" components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx

Length of output: 630

---

Remove focusability suggestion: no keyboard events are bound to this container

There are currently no keyboard event handlers (e.g., onKeyDown or addEventListener('keydown')) on this <div>, so adding tabIndex={0} won’t have any effect. If you intend to support keyboard navigation, first implement the necessary event handlers; only then should you make the container focusable.

Likely an incorrect or invalid review comment.