Skip to content

Commit 11ca60f

Browse files
Merge pull request #2738 from github/robertbrignull/useScrollIntoView
Convert ScrollIntoViewHelper into a React hook
2 parents 26b6023 + dea6bc2 commit 11ca60f

File tree

8 files changed

+50
-88
lines changed

8 files changed

+50
-88
lines changed

extensions/ql-vscode/src/view/results/AlertTable.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import {
1414
SarifInterpretationData,
1515
} from "../../common/interface-types";
1616
import { parseSarifLocation, isNoLocation } from "../../common/sarif-utils";
17-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
1817
import { sendTelemetry } from "../common/telemetry";
1918
import { AlertTableHeader } from "./AlertTableHeader";
2019
import { AlertTableNoResults } from "./AlertTableNoResults";
2120
import { AlertTableTruncatedMessage } from "./AlertTableTruncatedMessage";
2221
import { AlertTableResultRow } from "./AlertTableResultRow";
2322
import { useCallback, useEffect, useRef, useState } from "react";
23+
import { useScrollIntoView } from "./useScrollIntoView";
2424

2525
type AlertTableProps = ResultTableProps & {
2626
resultSet: InterpretedResultSet<SarifInterpretationData>;
@@ -29,17 +29,14 @@ type AlertTableProps = ResultTableProps & {
2929
export function AlertTable(props: AlertTableProps) {
3030
const { databaseUri, resultSet } = props;
3131

32-
const scroller = useRef<ScrollIntoViewHelper | undefined>(undefined);
33-
if (scroller.current === undefined) {
34-
scroller.current = new ScrollIntoViewHelper();
35-
}
36-
useEffect(() => scroller.current?.update());
37-
3832
const [expanded, setExpanded] = useState<Set<string>>(new Set<string>());
3933
const [selectedItem, setSelectedItem] = useState<Keys.ResultKey | undefined>(
4034
undefined,
4135
);
4236

37+
const selectedItemRef = useRef<any>();
38+
useScrollIntoView(selectedItem, selectedItemRef);
39+
4340
/**
4441
* Given a list of `keys`, toggle the first, and if we 'open' the
4542
* first item, open all the rest as well. This mimics vscode's file
@@ -162,7 +159,6 @@ export function AlertTable(props: AlertTableProps) {
162159
newExpanded.delete(Keys.keyToString(selectedItem));
163160
}
164161
}
165-
scroller.current?.scrollIntoViewOnNextUpdate();
166162
setExpanded(newExpanded);
167163
setSelectedItem(key);
168164
},
@@ -203,11 +199,11 @@ export function AlertTable(props: AlertTableProps) {
203199
resultIndex={resultIndex}
204200
expanded={expanded}
205201
selectedItem={selectedItem}
202+
selectedItemRef={selectedItemRef}
206203
databaseUri={databaseUri}
207204
sourceLocationPrefix={sourceLocationPrefix}
208205
updateSelectionCallback={updateSelectionCallback}
209206
toggleExpanded={toggle}
210-
scroller={scroller.current}
211207
/>
212208
),
213209
)}

extensions/ql-vscode/src/view/results/AlertTablePathNodeRow.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import * as Sarif from "sarif";
33
import * as Keys from "./result-keys";
44
import { SarifLocation } from "./locations/SarifLocation";
55
import { selectableZebraStripe } from "./result-table-utils";
6-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
76
import { useCallback, useMemo } from "react";
87

98
interface Props {
@@ -12,12 +11,12 @@ interface Props {
1211
pathIndex: number;
1312
resultIndex: number;
1413
selectedItem: undefined | Keys.ResultKey;
14+
selectedItemRef: React.RefObject<any>;
1515
databaseUri: string;
1616
sourceLocationPrefix: string;
1717
updateSelectionCallback: (
1818
resultKey: Keys.PathNode | Keys.Result | undefined,
1919
) => void;
20-
scroller?: ScrollIntoViewHelper;
2120
}
2221

2322
export function AlertTablePathNodeRow(props: Props) {
@@ -27,10 +26,10 @@ export function AlertTablePathNodeRow(props: Props) {
2726
pathIndex,
2827
resultIndex,
2928
selectedItem,
29+
selectedItemRef,
3030
databaseUri,
3131
sourceLocationPrefix,
3232
updateSelectionCallback,
33-
scroller,
3433
} = props;
3534

3635
const pathNodeKey: Keys.PathNode = useMemo(
@@ -51,7 +50,7 @@ export function AlertTablePathNodeRow(props: Props) {
5150
const zebraIndex = resultIndex + stepIndex;
5251
return (
5352
<tr
54-
ref={scroller?.ref(isSelected)}
53+
ref={isSelected ? selectedItemRef : undefined}
5554
className={isSelected ? "vscode-codeql__selected-path-node" : undefined}
5655
>
5756
<td className="vscode-codeql__icon-cell">

extensions/ql-vscode/src/view/results/AlertTablePathRow.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as React from "react";
22
import * as Sarif from "sarif";
33
import * as Keys from "./result-keys";
44
import { selectableZebraStripe } from "./result-table-utils";
5-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
65
import { AlertTablePathNodeRow } from "./AlertTablePathNodeRow";
76
import { AlertTableDropdownIndicatorCell } from "./AlertTableDropdownIndicatorCell";
87
import { useCallback, useMemo } from "react";
@@ -13,13 +12,13 @@ interface Props {
1312
resultIndex: number;
1413
currentPathExpanded: boolean;
1514
selectedItem: undefined | Keys.ResultKey;
15+
selectedItemRef: React.RefObject<any>;
1616
databaseUri: string;
1717
sourceLocationPrefix: string;
1818
updateSelectionCallback: (
1919
resultKey: Keys.PathNode | Keys.Result | undefined,
2020
) => void;
2121
toggleExpanded: (e: React.MouseEvent, keys: Keys.ResultKey[]) => void;
22-
scroller?: ScrollIntoViewHelper;
2322
}
2423

2524
export function AlertTablePathRow(props: Props) {
@@ -29,8 +28,8 @@ export function AlertTablePathRow(props: Props) {
2928
resultIndex,
3029
currentPathExpanded,
3130
selectedItem,
31+
selectedItemRef,
3232
toggleExpanded,
33-
scroller,
3433
} = props;
3534

3635
const pathKey = useMemo(
@@ -50,7 +49,7 @@ export function AlertTablePathRow(props: Props) {
5049
return (
5150
<>
5251
<tr
53-
ref={scroller?.ref(isPathSpecificallySelected)}
52+
ref={isPathSpecificallySelected ? selectedItemRef : undefined}
5453
{...selectableZebraStripe(isPathSpecificallySelected, resultIndex)}
5554
>
5655
<td className="vscode-codeql__icon-cell">

extensions/ql-vscode/src/view/results/AlertTableResultRow.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as React from "react";
22
import * as Sarif from "sarif";
33
import * as Keys from "./result-keys";
44
import { info, listUnordered } from "./octicons";
5-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
65
import { selectableZebraStripe } from "./result-table-utils";
76
import { AlertTableDropdownIndicatorCell } from "./AlertTableDropdownIndicatorCell";
87
import { useCallback, useMemo } from "react";
@@ -15,13 +14,13 @@ interface Props {
1514
resultIndex: number;
1615
expanded: Set<string>;
1716
selectedItem: undefined | Keys.ResultKey;
17+
selectedItemRef: React.RefObject<any>;
1818
databaseUri: string;
1919
sourceLocationPrefix: string;
2020
updateSelectionCallback: (
2121
resultKey: Keys.PathNode | Keys.Result | undefined,
2222
) => void;
2323
toggleExpanded: (e: React.MouseEvent, keys: Keys.ResultKey[]) => void;
24-
scroller?: ScrollIntoViewHelper;
2524
}
2625

2726
export function AlertTableResultRow(props: Props) {
@@ -30,11 +29,11 @@ export function AlertTableResultRow(props: Props) {
3029
resultIndex,
3130
expanded,
3231
selectedItem,
32+
selectedItemRef,
3333
databaseUri,
3434
sourceLocationPrefix,
3535
updateSelectionCallback,
3636
toggleExpanded,
37-
scroller,
3837
} = props;
3938

4039
const resultKey: Keys.Result = useMemo(
@@ -81,7 +80,7 @@ export function AlertTableResultRow(props: Props) {
8180
return (
8281
<>
8382
<tr
84-
ref={scroller?.ref(resultRowIsSelected)}
83+
ref={resultRowIsSelected ? selectedItemRef : undefined}
8584
{...selectableZebraStripe(resultRowIsSelected, resultIndex)}
8685
>
8786
{result.codeFlows === undefined ? (

extensions/ql-vscode/src/view/results/RawTable.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import RawTableRow from "./RawTableRow";
1313
import { ResultRow } from "../../common/bqrs-cli-types";
1414
import { onNavigation } from "./ResultsApp";
1515
import { tryGetResolvableLocation } from "../../common/bqrs-utils";
16-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
1716
import { sendTelemetry } from "../common/telemetry";
1817
import { assertNever } from "../../common/helpers-pure";
1918
import { EmptyQueryResultsMessage } from "./EmptyQueryResultsMessage";
19+
import { useScrollIntoView } from "./useScrollIntoView";
2020

2121
type RawTableProps = {
2222
databaseUri: string;
@@ -38,11 +38,8 @@ export function RawTable({
3838
}: RawTableProps) {
3939
const [selectedItem, setSelectedItem] = useState<TableItem | undefined>();
4040

41-
const scroller = useRef<ScrollIntoViewHelper | undefined>(undefined);
42-
if (scroller.current === undefined) {
43-
scroller.current = new ScrollIntoViewHelper();
44-
}
45-
useEffect(() => scroller.current?.update());
41+
const selectedItemRef = useRef<any>();
42+
useScrollIntoView(selectedItem, selectedItemRef);
4643

4744
const setSelection = useCallback((row: number, column: number): void => {
4845
setSelectedItem({ row, column });
@@ -76,11 +73,10 @@ export function RawTable({
7673
jumpToLocation(location, databaseUri);
7774
}
7875
}
79-
scroller.current?.scrollIntoViewOnNextUpdate();
8076
return { row: nextRow, column: nextColumn };
8177
});
8278
},
83-
[databaseUri, resultSet, scroller],
79+
[databaseUri, resultSet],
8480
);
8581

8682
const handleNavigationEvent = useCallback(
@@ -140,7 +136,7 @@ export function RawTable({
140136
selectedItem?.row === rowIndex ? selectedItem?.column : undefined
141137
}
142138
onSelected={setSelection}
143-
scroller={scroller.current}
139+
selectedItemRef={selectedItemRef}
144140
/>
145141
));
146142

extensions/ql-vscode/src/view/results/RawTableRow.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,15 @@ import * as React from "react";
22
import { ResultRow } from "../../common/bqrs-cli-types";
33
import { selectedRowClassName, zebraStripe } from "./result-table-utils";
44
import RawTableValue from "./RawTableValue";
5-
import { ScrollIntoViewHelper } from "./scroll-into-view-helper";
65

76
interface Props {
87
rowIndex: number;
98
row: ResultRow;
109
databaseUri: string;
1110
className?: string;
1211
selectedColumn?: number;
12+
selectedItemRef?: React.Ref<any>;
1313
onSelected?: (row: number, column: number) => void;
14-
scroller?: ScrollIntoViewHelper;
1514
}
1615

1716
export default function RawTableRow(props: Props) {
@@ -26,7 +25,7 @@ export default function RawTableRow(props: Props) {
2625
const isSelected = props.selectedColumn === columnIndex;
2726
return (
2827
<td
29-
ref={props.scroller?.ref(isSelected)}
28+
ref={isSelected ? props.selectedItemRef : undefined}
3029
key={columnIndex}
3130
{...(isSelected ? { className: selectedRowClassName } : {})}
3231
>

extensions/ql-vscode/src/view/results/scroll-into-view-helper.ts

Lines changed: 0 additions & 55 deletions
This file was deleted.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { RefObject, useEffect } from "react";
2+
3+
export function useScrollIntoView<T>(
4+
selectedElement: T | undefined,
5+
selectedElementRef: RefObject<any>,
6+
) {
7+
useEffect(() => {
8+
const element = selectedElementRef.current as HTMLElement | undefined;
9+
if (!element) {
10+
return;
11+
}
12+
const rect = element.getBoundingClientRect();
13+
// The selected item's bounding box might be on screen, but hidden underneath the sticky header
14+
// which overlaps the table view. As a workaround we hardcode a fixed distance from the top which
15+
// we consider to be obscured. It does not have to exact, as it's just a threshold for when to scroll.
16+
const heightOfStickyHeader = 30;
17+
if (rect.top < heightOfStickyHeader || rect.bottom > window.innerHeight) {
18+
element.scrollIntoView({
19+
block: "center", // vertically align to center
20+
});
21+
}
22+
if (rect.left < 0 || rect.right > window.innerWidth) {
23+
element.scrollIntoView({
24+
block: "nearest",
25+
inline: "nearest", // horizontally align as little as possible
26+
});
27+
}
28+
}, [selectedElement, selectedElementRef]);
29+
}

0 commit comments

Comments
 (0)