Skip to content

Commit ca6f981

Browse files
Marshall PetersonMarshall Peterson
authored andcommitted
fixed virtualizer bug root cause and removed aria dependency in stately
1 parent 452d284 commit ca6f981

File tree

7 files changed

+29
-42
lines changed

7 files changed

+29
-42
lines changed

packages/@react-spectrum/table/src/Resizer.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import {FocusRing} from '@react-aria/focus';
44
import React from 'react';
55
import styles from '@adobe/spectrum-css-temp/components/table/vars.css';
66
import {useTableColumnResize} from '@react-aria/table/src/useTableColumnResize';
7+
import {useTableContext} from './TableView';
78

89

910
function Resizer(props, ref) {
10-
const {state, item} = props;
11+
const {item} = props;
12+
let state = useTableContext();
1113
let {resizerProps} = useTableColumnResize(state, item, ref);
1214
return (
1315
<FocusRing focusRingClass={classNames(styles, 'focus-ring')}>

packages/@react-spectrum/table/src/TableView.tsx

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ const SELECTION_CELL_DEFAULT_WIDTH = {
8080
};
8181

8282
const TableContext = React.createContext<TableState<unknown>>(null);
83-
function useTableContext() {
83+
export function useTableContext() {
8484
return useContext(TableContext);
8585
}
8686

@@ -131,13 +131,13 @@ function TableView<T extends object>(props: SpectrumTableProps<T>, ref: DOMRef<H
131131
: DEFAULT_HEADER_HEIGHT[scale],
132132
estimatedHeadingHeight: props.overflowMode === 'wrap'
133133
? DEFAULT_HEADER_HEIGHT[scale]
134-
: null,
135-
getColumnWidth: state.getColumnWidth
134+
: null
136135
}),
137-
[props.overflowMode, scale, density, state.getColumnWidth]
136+
[props.overflowMode, scale, density]
138137
);
139138
let {direction} = useLocale();
140139
layout.collection = state.collection;
140+
layout.getColumnWidth = state.getColumnWidth;
141141

142142
let {gridProps} = useTable({
143143
...props,
@@ -309,13 +309,14 @@ function TableView<T extends object>(props: SpectrumTableProps<T>, ref: DOMRef<H
309309
renderView={renderView}
310310
renderWrapper={renderWrapper}
311311
setTableWidth={state.setTableWidth}
312-
domRef={domRef} />
312+
domRef={domRef}
313+
getColumnWidth={state.getColumnWidth} />
313314
</TableContext.Provider>
314315
);
315316
}
316317

317318
// This is a custom Virtualizer that also has a header that syncs its scroll position with the body.
318-
function TableVirtualizer({layout, collection, focusedKey, renderView, renderWrapper, domRef, setTableWidth, ...otherProps}) {
319+
function TableVirtualizer({layout, collection, focusedKey, renderView, renderWrapper, domRef, setTableWidth, getColumnWidth, ...otherProps}) {
319320
let {direction} = useLocale();
320321
let headerRef = useRef<HTMLDivElement>();
321322
let bodyRef = useRef<HTMLDivElement>();
@@ -352,6 +353,11 @@ function TableVirtualizer({layout, collection, focusedKey, renderView, renderWra
352353
}
353354
}, state, domRef);
354355

356+
// If columnwidths change, need to relayout.
357+
useLayoutEffect(() => {
358+
state.virtualizer.relayoutNow({sizeChanged: true});
359+
}, [getColumnWidth, state.virtualizer]);
360+
355361
let headerHeight = layout.getLayoutInfo('header')?.rect.height || 0;
356362
let visibleRect = state.virtualizer.visibleRect;
357363

@@ -532,7 +538,7 @@ function ResizableTableColumnHeader({item, state}) {
532538
</Item>
533539
</Menu>
534540
</MenuTrigger>
535-
<Resizer ref={ref} state={state} item={item} />
541+
<Resizer ref={ref} item={item} />
536542
</>
537543
);
538544
}

packages/@react-spectrum/table/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
/// <reference types="css-module-types" />
1414

15-
export * from './TableView';
15+
export {TableView} from './TableView';
1616
import {Column} from '@react-stately/table';
1717
import {SpectrumColumnProps} from '@react-types/table';
1818

packages/@react-stately/layout/src/TableLayout.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ import {LayoutNode, ListLayout, ListLayoutOptions} from './ListLayout';
1717
import {TableCollection} from '@react-types/table';
1818

1919

20-
type TableLayoutOptions<T> = ListLayoutOptions<T> & {
21-
getColumnWidth: (key: Key) => number
22-
}
23-
2420
export class TableLayout<T> extends ListLayout<T> {
2521
collection: TableCollection<T>;
2622
lastCollection: TableCollection<T>;
@@ -29,9 +25,8 @@ export class TableLayout<T> extends ListLayout<T> {
2925
wasLoading = false;
3026
isLoading = false;
3127

32-
constructor(options: TableLayoutOptions<T>) {
28+
constructor(options: ListLayoutOptions<T>) {
3329
super(options);
34-
this.getColumnWidth = options.getColumnWidth;
3530
this.stickyColumnIndices = [];
3631
}
3732

packages/@react-stately/table/src/useTableColumnResizeState.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {ColumnProps} from '@react-types/table';
33
import {getContentWidth, getDynamicColumnWidths, getMaxWidth, getMinWidth, isStatic, parseStaticWidth} from './utils';
44
import {GridNode} from '@react-types/grid';
55
import {Key, MutableRefObject, useCallback, useRef, useState} from 'react';
6-
import {useLayoutEffect} from '@react-aria/utils';
76

87
export interface AffectedColumnWidth {
98
/** The column key. */
@@ -49,7 +48,7 @@ export interface ColumnResizeStateProps<T> {
4948

5049
export function useTableColumnResizeState<T>(props: ColumnResizeStateProps<T>): ColumnResizeState<T> {
5150
const {columns, getDefaultWidth, tableWidth: defaultTableWidth = null} = props;
52-
const columnsRef = useRef<GridNode<T>[]>();
51+
const columnsRef = useRef<GridNode<T>[]>([]);
5352
const tableWidth = useRef<number>(defaultTableWidth);
5453
const isResizing = useRef<boolean>(null);
5554
const startResizeContentWidth = useRef<number>();
@@ -103,12 +102,15 @@ export function useTableColumnResizeState<T>(props: ColumnResizeStateProps<T>):
103102
return widths;
104103
}, [getStaticAndDynamicColumns, getResolvedColumnWidth]);
105104

105+
106+
const prevColKeys = columnsRef.current.map(col => col.key);
107+
const colKeys = columns.map(col => col.key);
106108
// if the columns change, need to rebuild widths.
107-
useLayoutEffect(() => {
109+
if (!colKeys.every((col, i) => col === prevColKeys[i])) {
108110
columnsRef.current = columns;
109111
const widths = buildColumnWidths(columns, tableWidth.current);
110112
setColumnWidthsForRef(widths);
111-
}, [columns, buildColumnWidths]);
113+
}
112114

113115
function setTableWidth(width: number) {
114116
if (width && width !== tableWidth.current) {

packages/@react-stately/virtualizer/src/ReusableView.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export class ReusableView<T extends object, V> {
3131
layoutInfo: LayoutInfo | null;
3232

3333
/** The content currently being displayed by this view, set by the collection view. */
34-
content: any;
34+
content: T;
3535

3636
rendered: V;
3737

packages/@react-stately/virtualizer/src/Virtualizer.ts

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,9 @@ export class Virtualizer<T extends object, V, W> {
326326
reusableView.rendered = this._renderContent(type, reusableView.content);
327327
}
328328

329-
private _renderContent(type: string, content: any) {
330-
let cached = this._renderedContent.get(content);
331-
// always need to rerender columns so that the resizer aria-values gets updated correctly
332-
const isResizableColumn = type === 'column' && content.props?.allowsResizing;
333-
if (cached != null && !isResizableColumn) {
329+
private _renderContent(type: string, content: T) {
330+
let cached = this._renderedContent.get(content);
331+
if (cached != null) {
334332
return cached;
335333
}
336334

@@ -663,9 +661,7 @@ export class Virtualizer<T extends object, V, W> {
663661
}
664662

665663
let item = this.getItem(visibleLayoutInfos.get(key).key);
666-
// always need to rerender columns so that the resizer aria-values get updated
667-
const isResizableColumn = view.viewType === 'column' && view.content.props?.allowsResizing;
668-
if (view.content === item && !isResizableColumn) {
664+
if (view.content === item) {
669665
toUpdate.delete(key);
670666
} else {
671667
// If the view type changes, delete and recreate the view instead of updating
@@ -747,11 +743,6 @@ export class Virtualizer<T extends object, V, W> {
747743

748744
for (let key of toUpdate) {
749745
let view = currentlyVisible.get(key) as ReusableView<T, V>;
750-
if (!view.layoutInfo) {
751-
let layoutInfo = visibleLayoutInfos.get(key);
752-
view.layoutInfo = layoutInfo;
753-
}
754-
755746
this._renderedContent.delete(key);
756747
this._renderView(view);
757748
}
@@ -791,9 +782,6 @@ export class Virtualizer<T extends object, V, W> {
791782
// method to build the final tree.
792783
let viewsByParentKey = new Map([[null, []]]);
793784
for (let view of this._children) {
794-
if (!view.layoutInfo) {
795-
return;
796-
}
797785
if (!viewsByParentKey.has(view.layoutInfo.parentKey)) {
798786
viewsByParentKey.set(view.layoutInfo.parentKey, []);
799787
}
@@ -845,9 +833,6 @@ export class Virtualizer<T extends object, V, W> {
845833
if (this._transaction) {
846834
for (let view of this._transaction.toRemove.values()) {
847835
let cur = view.layoutInfo;
848-
if (!cur) {
849-
return;
850-
}
851836
let layoutInfo = this.layout.getLayoutInfo(cur.key);
852837
if (this._applyLayoutInfo(view, layoutInfo)) {
853838
updated = true;
@@ -856,9 +841,6 @@ export class Virtualizer<T extends object, V, W> {
856841

857842
for (let view of this._transaction.removed.values()) {
858843
let cur = view.layoutInfo;
859-
if (!cur) {
860-
return;
861-
}
862844
let layoutInfo = this._transaction.finalLayoutInfo.get(cur.key) || cur;
863845
layoutInfo = this.layout.getFinalLayoutInfo(layoutInfo.copy());
864846
if (this._applyLayoutInfo(view, layoutInfo)) {

0 commit comments

Comments
 (0)