From 6bbe73cfa6987a48f250637979e7092b11845c96 Mon Sep 17 00:00:00 2001 From: Yihui Liao Date: Tue, 7 Nov 2023 15:42:18 -0800 Subject: [PATCH 01/10] tableview crash bug --- .../table/stories/Table.stories.tsx | 272 +++++++++++++++++- 1 file changed, 270 insertions(+), 2 deletions(-) diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 96b65549fb1..2e6a734c645 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -16,6 +16,7 @@ import Add from '@spectrum-icons/workflow/Add'; import {Breadcrumbs, Item} from '@react-spectrum/breadcrumbs'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Cell, Column, Row, SpectrumTableProps, TableBody, TableHeader, TableView} from '../'; +import {Checkbox} from '@react-spectrum/checkbox'; import {ComponentMeta, ComponentStoryObj} from '@storybook/react'; import {Content, View} from '@react-spectrum/view'; import {ControllingResize, PokemonColumn} from './ControllingResize'; @@ -927,8 +928,7 @@ function AsyncLoadingExample(props) { let url = new URL('https://www.reddit.com/r/upliftingnews.json'); if (cursor) { url.searchParams.append('after', cursor); - } - + } let res = await fetch(url.toString(), {signal}); let json = await res.json(); return {items: json.data.children, cursor: json.data.after}; @@ -983,6 +983,274 @@ export const AsyncLoading: TableStory = { name: 'async loading' }; +async function fakeFetch() { + return new Promise( + (resolve, reject) => { + console.log('fetching data'); + setTimeout(() => resolve({json: async function () { + return {data: {children: [ + {data: {id: 'foo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: 'fooo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: 'foooo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: 'fooooo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: 'foooooo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: 'doo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '1', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '2', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '3', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '4', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '5', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, + {data: {id: '6', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}} + ]}} + }}), 1000); + } + ); +} + +async function load(signal, cursor) { + // const url = new URL('https://www.reddit.com/r/aww.json'); + + // if (cursor) { + // url.searchParams.append('after', cursor); + // } + + // // const res = await fetch(url.toString(), { + // // signal + // // }); + // const res = await fakeFetch(); + // const json = await res.json(); + // const items = json.data.children.map((item) => { + // return { + // id: item.data.id, + // preview: item.data.thumbnail, + // ups: item.data.ups, + // title: item.data.title, + // created: new Date(item.data.created * 1000).toISOString() + // }; + // }); + + // console.log('load'); + + // return { + // items, + // cursor: json.data.after, + // total: 987 + // }; + + const url = new URL(`https://www.reddit.com/r/aww.json`); + + if (cursor) { + url.searchParams.append('after', cursor); + } + + const res = await fetch(url.toString(), { + signal + }); + const json = await res.json(); + const items = json.data.children.map((item) => { + return { + id: item.data.id, + preview: item.data.thumbnail, + ups: item.data.ups, + title: item.data.title, + created: new Date(item.data.created * 1000).toISOString(), + }; + }); + return { + items, + cursor: json.data.after, + total: 987 + }; +} + +async function sort({items, sortDescriptor}) { + return { + items: items.slice().sort((a, b) => { + let cmp = a[sortDescriptor.column] < b[sortDescriptor.column] ? -1 : 1; + + if (sortDescriptor.direction === 'descending') { + cmp *= -1; + } + + return cmp; + }) + + }; +} + +export const AsyncLoadingQuarryTest: TableStory = { + args: { + 'aria-label': 'Top news from Reddit', + selectionMode: 'multiple', + height: 400, + width: '100%' + }, + render: (args) => , + name: 'async loading quarry test' +}; + +function AsyncLoadingExampleQuarryTest(props) { + let [filters, setFilters] = React.useState({}); + // let [total, setTotal] = useState(null); + // let [page, setPage] = useState(initialPage); + // let [limit, setLimit] = useState(initialLimit); + + const rColumns = [ + { + key: 'title', + label: 'Title' + }, + { + key: 'ups', + label: 'Upvotes', + width: 200 + }, + { + key: 'created', + label: 'Created', + width: 350 + } + ]; + interface Post { + id: string, + key: string, + preview?: string, + ups: number, + title: string, + created: string, + url: string + } + + // const filtersSort = useCallback( + // async (opts) => { + // return sort({filters, ...opts}); + // }, + // [filters, sort], + // ); + + // const pagedLoad = useCallback( + // async (opts) => { + // // const {pagination} = filters; + // // const pageToLoad = pagination?.page ?? page; + // // const limitToLoad = pagination?.limit ?? limit; + + // // const {total, ...data} = await load({ filters, limit: limitToLoad, page: pageToLoad, ...opts }); + // const {total, ...data} = await load({}); + + + // if (total != null) { + // setTotal(total); + // } + // if (pageToLoad != null) { + // setPage(pageToLoad); + // } + + // if (limitToLoad != null) { + // setLimit(limitToLoad); + // } + + // return {...data}; + // }, + // [filters, load], + // ); + + // const listOpts = { + // getKey: (item) => item.id, + // load: pagedLoad, + // ...(sort && {sort: filtersSort}) + // } + + + // let list = useAsyncList(listOpts); + + + let list = useAsyncList({ + getKey: (item) => item.id, + async load({signal, cursor}) { + console.log('load'); + + const url = new URL(`https://www.reddit.com/r/aww.json`); + + if (cursor) { + url.searchParams.append('after', cursor); + } + + const res = await fakeFetch(); + const json = await res.json(); + const items = json.data.children.map((item) => { + return { + id: item.data.id, + preview: item.data.thumbnail, + ups: item.data.ups, + title: item.data.title, + created: new Date(item.data.created * 1000).toISOString(), + }; + }); + return { + items, + cursor: json.data.after, + total: 987 + }; + }, + async sort({items, sortDescriptor}) { + return { + items: items.slice().sort((a, b) => { + let cmp = a[sortDescriptor.column] < b[sortDescriptor.column] ? -1 : 1; + + if (sortDescriptor.direction === 'descending') { + cmp *= -1; + } + + return cmp; + }) + + }; + } + }); + + let reloadDeps = [filters]; + + useMountEffect((): void => { + list.reload(); + }, reloadDeps); + + console.log('reloadDeps', reloadDeps); + + return ( + + + {(column) => ( + + {column.label} + + )} + + + {item => + ( + {key => + key === 'title' + ? {item.title} + : {item[key]} + } + ) + } + + + ); +} + +function useMountEffect(fn: () => void, deps: Array): void { + const mounted = React.useRef(false); + React.useEffect(() => { + if (mounted.current) { + fn(); + } else { + mounted.current = true; + } + }, deps); +} + export const HideHeader: TableStory = { args: { 'aria-label': 'TableView with static contents', From ad6859e1b15ec3a4bf177930e4ef44e64c4b720b Mon Sep 17 00:00:00 2001 From: Yihui Liao Date: Tue, 7 Nov 2023 15:55:01 -0800 Subject: [PATCH 02/10] clean up --- .../table/stories/Table.stories.tsx | 117 ------------------ 1 file changed, 117 deletions(-) diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 2e6a734c645..a5ed6db9d4a 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -1007,77 +1007,6 @@ async function fakeFetch() { ); } -async function load(signal, cursor) { - // const url = new URL('https://www.reddit.com/r/aww.json'); - - // if (cursor) { - // url.searchParams.append('after', cursor); - // } - - // // const res = await fetch(url.toString(), { - // // signal - // // }); - // const res = await fakeFetch(); - // const json = await res.json(); - // const items = json.data.children.map((item) => { - // return { - // id: item.data.id, - // preview: item.data.thumbnail, - // ups: item.data.ups, - // title: item.data.title, - // created: new Date(item.data.created * 1000).toISOString() - // }; - // }); - - // console.log('load'); - - // return { - // items, - // cursor: json.data.after, - // total: 987 - // }; - - const url = new URL(`https://www.reddit.com/r/aww.json`); - - if (cursor) { - url.searchParams.append('after', cursor); - } - - const res = await fetch(url.toString(), { - signal - }); - const json = await res.json(); - const items = json.data.children.map((item) => { - return { - id: item.data.id, - preview: item.data.thumbnail, - ups: item.data.ups, - title: item.data.title, - created: new Date(item.data.created * 1000).toISOString(), - }; - }); - return { - items, - cursor: json.data.after, - total: 987 - }; -} - -async function sort({items, sortDescriptor}) { - return { - items: items.slice().sort((a, b) => { - let cmp = a[sortDescriptor.column] < b[sortDescriptor.column] ? -1 : 1; - - if (sortDescriptor.direction === 'descending') { - cmp *= -1; - } - - return cmp; - }) - - }; -} - export const AsyncLoadingQuarryTest: TableStory = { args: { 'aria-label': 'Top news from Reddit', @@ -1091,9 +1020,6 @@ export const AsyncLoadingQuarryTest: TableStory = { function AsyncLoadingExampleQuarryTest(props) { let [filters, setFilters] = React.useState({}); - // let [total, setTotal] = useState(null); - // let [page, setPage] = useState(initialPage); - // let [limit, setLimit] = useState(initialLimit); const rColumns = [ { @@ -1121,49 +1047,6 @@ function AsyncLoadingExampleQuarryTest(props) { url: string } - // const filtersSort = useCallback( - // async (opts) => { - // return sort({filters, ...opts}); - // }, - // [filters, sort], - // ); - - // const pagedLoad = useCallback( - // async (opts) => { - // // const {pagination} = filters; - // // const pageToLoad = pagination?.page ?? page; - // // const limitToLoad = pagination?.limit ?? limit; - - // // const {total, ...data} = await load({ filters, limit: limitToLoad, page: pageToLoad, ...opts }); - // const {total, ...data} = await load({}); - - - // if (total != null) { - // setTotal(total); - // } - // if (pageToLoad != null) { - // setPage(pageToLoad); - // } - - // if (limitToLoad != null) { - // setLimit(limitToLoad); - // } - - // return {...data}; - // }, - // [filters, load], - // ); - - // const listOpts = { - // getKey: (item) => item.id, - // load: pagedLoad, - // ...(sort && {sort: filtersSort}) - // } - - - // let list = useAsyncList(listOpts); - - let list = useAsyncList({ getKey: (item) => item.id, async load({signal, cursor}) { From 29d04e8807687040918620fee0a61ae1c2167866 Mon Sep 17 00:00:00 2001 From: Yihui Liao Date: Tue, 7 Nov 2023 16:16:13 -0800 Subject: [PATCH 03/10] lint --- .../table/stories/Table.stories.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index a5ed6db9d4a..21e1417b952 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -16,7 +16,6 @@ import Add from '@spectrum-icons/workflow/Add'; import {Breadcrumbs, Item} from '@react-spectrum/breadcrumbs'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Cell, Column, Row, SpectrumTableProps, TableBody, TableHeader, TableView} from '../'; -import {Checkbox} from '@react-spectrum/checkbox'; import {ComponentMeta, ComponentStoryObj} from '@storybook/react'; import {Content, View} from '@react-spectrum/view'; import {ControllingResize, PokemonColumn} from './ControllingResize'; @@ -985,7 +984,7 @@ export const AsyncLoading: TableStory = { async function fakeFetch() { return new Promise( - (resolve, reject) => { + (resolve) => { console.log('fetching data'); setTimeout(() => resolve({json: async function () { return {data: {children: [ @@ -1001,7 +1000,7 @@ async function fakeFetch() { {data: {id: '4', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, {data: {id: '5', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, {data: {id: '6', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}} - ]}} + ]}}; }}), 1000); } ); @@ -1049,10 +1048,10 @@ function AsyncLoadingExampleQuarryTest(props) { let list = useAsyncList({ getKey: (item) => item.id, - async load({signal, cursor}) { + async load({cursor}) { console.log('load'); - const url = new URL(`https://www.reddit.com/r/aww.json`); + const url = new URL('https://www.reddit.com/r/aww.json'); if (cursor) { url.searchParams.append('after', cursor); @@ -1066,7 +1065,7 @@ function AsyncLoadingExampleQuarryTest(props) { preview: item.data.thumbnail, ups: item.data.ups, title: item.data.title, - created: new Date(item.data.created * 1000).toISOString(), + created: new Date(item.data.created * 1000).toISOString() }; }); return { @@ -1097,10 +1096,8 @@ function AsyncLoadingExampleQuarryTest(props) { list.reload(); }, reloadDeps); - console.log('reloadDeps', reloadDeps); - return ( - + {(column) => ( From 615aaaa9084676cce254c23a29993d437ee0c7da Mon Sep 17 00:00:00 2001 From: Yihui Liao Date: Thu, 9 Nov 2023 12:41:25 -0800 Subject: [PATCH 04/10] subtract border width from body width --- packages/@react-spectrum/table/src/TableViewBase.tsx | 5 +++-- packages/@react-spectrum/table/stories/Table.stories.tsx | 6 ++---- packages/@react-stately/layout/src/TableLayout.ts | 7 +++++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index 28d3cbaf88b..286c10f1855 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -228,11 +228,12 @@ function TableViewBase(props: TableBaseProps, ref: DOMRef { - console.log('fetching data'); setTimeout(() => resolve({json: async function () { return {data: {children: [ {data: {id: 'foo', thumbnail: 'https://i.imgur.com/Z7AzH2c.jpg', ups: '7', title: 'cats', created: Date.now()}}, @@ -1014,7 +1013,7 @@ export const AsyncLoadingQuarryTest: TableStory = { width: '100%' }, render: (args) => , - name: 'async loading quarry test' + name: 'async reload on sort' }; function AsyncLoadingExampleQuarryTest(props) { @@ -1049,8 +1048,6 @@ function AsyncLoadingExampleQuarryTest(props) { let list = useAsyncList({ getKey: (item) => item.id, async load({cursor}) { - console.log('load'); - const url = new URL('https://www.reddit.com/r/aww.json'); if (cursor) { @@ -1058,6 +1055,7 @@ function AsyncLoadingExampleQuarryTest(props) { } const res = await fakeFetch(); + // @ts-ignore const json = await res.json(); const items = json.data.children.map((item) => { return { diff --git a/packages/@react-stately/layout/src/TableLayout.ts b/packages/@react-stately/layout/src/TableLayout.ts index fd15f170d3d..c65c1cc3bc2 100644 --- a/packages/@react-stately/layout/src/TableLayout.ts +++ b/packages/@react-stately/layout/src/TableLayout.ts @@ -21,7 +21,8 @@ import {TableColumnLayout} from '@react-stately/table'; type TableLayoutOptions = ListLayoutOptions & { columnLayout: TableColumnLayout, - initialCollection: TableCollection + initialCollection: TableCollection, + bodyBorderWidth: number } export class TableLayout extends ListLayout { @@ -39,6 +40,7 @@ export class TableLayout extends ListLayout { uncontrolledColumns: Map>; uncontrolledWidths: Map; resizingColumn: Key | null; + bodyBorderWidth = 0; constructor(options: TableLayoutOptions) { super(options); @@ -50,6 +52,7 @@ export class TableLayout extends ListLayout { this.controlledColumns = controlledColumns; this.uncontrolledColumns = uncontrolledColumns; this.uncontrolledWidths = this.columnLayout.getInitialUncontrolledWidths(uncontrolledColumns); + this.bodyBorderWidth = options.bodyBorderWidth; } protected shouldInvalidateEverything(invalidationContext: InvalidationContext, unknown>): boolean { @@ -141,7 +144,7 @@ export class TableLayout extends ListLayout { let body = this.buildBody(0); this.lastPersistedKeys = null; - body.layoutInfo.rect.width = Math.max(header.layoutInfo.rect.width, body.layoutInfo.rect.width); + body.layoutInfo.rect.width = Math.max(header.layoutInfo.rect.width - (2 * this.bodyBorderWidth), body.layoutInfo.rect.width); this.contentSize = new Size(body.layoutInfo.rect.width, body.layoutInfo.rect.maxY); return [ header, From 03c4a8b1b4e17c675c0c2320a3f3992115943e65 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 9 Nov 2023 17:21:09 -0800 Subject: [PATCH 05/10] Use percentage width when elements are full width of their parents --- .../virtualizer/src/ScrollView.tsx | 13 +++++++++++- .../virtualizer/src/VirtualizerItem.tsx | 20 +++++++++++++++---- .../virtualizer/src/useVirtualizerItem.ts | 1 + .../table/src/TableViewBase.tsx | 2 +- .../@react-stately/layout/src/TableLayout.ts | 2 +- packages/@react-types/sidenav/src/index.d.ts | 1 + 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index 526996f19eb..de7f24587f9 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -166,9 +166,20 @@ function ScrollView(props: ScrollViewProps, ref: RefObject) { style.overflow = 'auto'; } + innerStyle = { + // Omit width on inner element if it is equal to the width of the outer element. + // Since this is a block element, the browser will automatically make it full width. + // This prevents horizontal scrollbars from flickering when not needed. + width: contentSize.width === state.width ? undefined : contentSize.width, + height: contentSize.height, + pointerEvents: isScrolling ? 'none' : 'auto', + position: 'relative', + ...innerStyle + }; + return (
-
+
{children}
diff --git a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx index 066f918ee88..005642386c4 100644 --- a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx +++ b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx @@ -11,7 +11,7 @@ */ import {Direction} from '@react-types/shared'; -import {LayoutInfo} from '@react-stately/virtualizer'; +import {LayoutInfo, Size} from '@react-stately/virtualizer'; import React, {CSSProperties, ReactNode, useRef} from 'react'; import {useLocale} from '@react-aria/i18n'; import {useVirtualizerItem, VirtualizerItemOptions} from './useVirtualizerItem'; @@ -33,14 +33,19 @@ export function VirtualizerItem(props: VirtualizerItemProps) { }); return ( -
+
{children}
); } let cache = new WeakMap(); -export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent?: LayoutInfo | null): CSSProperties { +export function layoutInfoToStyle( + layoutInfo: LayoutInfo, + dir: Direction, + parent?: LayoutInfo | null, + contentSize?: Size +): CSSProperties { let xProperty = dir === 'rtl' ? 'right' : 'left'; let cached = cache.get(layoutInfo); if (cached && cached[xProperty] != null) { @@ -56,6 +61,13 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent } } + // Determine if the layoutInfo is the full width of its parent. + // In this case, we use 100% as the width rather than a pixel value, + // which avoids issues with flickering scrollbars. + let isFullWidth = parent + ? layoutInfo.rect.x === parent.rect.x && layoutInfo.rect.width === parent.rect.width + : contentSize && layoutInfo.rect.x === 0 && layoutInfo.rect.width === contentSize.width; + let style: CSSProperties = { position: layoutInfo.isSticky ? 'sticky' : 'absolute', // Sticky elements are positioned in normal document flow. Display inline-block so that they don't push other sticky columns onto the following rows. @@ -67,7 +79,7 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent WebkitTransition: 'all', WebkitTransitionDuration: 'inherit', transitionDuration: 'inherit', - width: layoutInfo.rect.width, + width: isFullWidth ? '100%' : layoutInfo.rect.width, height: layoutInfo.rect.height, opacity: layoutInfo.opacity, zIndex: layoutInfo.zIndex, diff --git a/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts b/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts index 848027f0b0b..f537d6f9454 100644 --- a/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts +++ b/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts @@ -15,6 +15,7 @@ import {LayoutInfo, Size} from '@react-stately/virtualizer'; import {useLayoutEffect} from '@react-aria/utils'; interface IVirtualizer { + contentSize: Size, updateItemSize(key: Key, size: Size): void } diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index 286c10f1855..84964dee5c4 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -287,7 +287,7 @@ function TableViewBase(props: TableBaseProps, ref: DOMRef, ReactNode>; let renderWrapper = (parent: View, reusableView: View, children: View[], renderChildren: (views: View[]) => ReactElement[]) => { - let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent && parent.layoutInfo); + let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent?.layoutInfo, reusableView.virtualizer.contentSize); if (style.overflow === 'hidden') { style.overflow = 'visible'; // needed to support position: sticky } diff --git a/packages/@react-stately/layout/src/TableLayout.ts b/packages/@react-stately/layout/src/TableLayout.ts index c65c1cc3bc2..3c3f5918fb4 100644 --- a/packages/@react-stately/layout/src/TableLayout.ts +++ b/packages/@react-stately/layout/src/TableLayout.ts @@ -144,7 +144,7 @@ export class TableLayout extends ListLayout { let body = this.buildBody(0); this.lastPersistedKeys = null; - body.layoutInfo.rect.width = Math.max(header.layoutInfo.rect.width - (2 * this.bodyBorderWidth), body.layoutInfo.rect.width); + body.layoutInfo.rect.width = Math.max(header.layoutInfo.rect.width, body.layoutInfo.rect.width); this.contentSize = new Size(body.layoutInfo.rect.width, body.layoutInfo.rect.maxY); return [ header, diff --git a/packages/@react-types/sidenav/src/index.d.ts b/packages/@react-types/sidenav/src/index.d.ts index a9dfc3c8388..cd429a1e39b 100644 --- a/packages/@react-types/sidenav/src/index.d.ts +++ b/packages/@react-types/sidenav/src/index.d.ts @@ -28,6 +28,7 @@ export interface SpectrumSideNavItemProps extends HTMLAttributes } interface IVirtualizer { + contentSize: Size, updateItemSize(key: Key, size: Size): void } From dffacfaf3b730b1bd853c151462b211101b44575 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 10 Nov 2023 14:18:20 +1100 Subject: [PATCH 06/10] remove old code --- packages/@react-spectrum/table/src/TableViewBase.tsx | 5 ++--- packages/@react-stately/layout/src/TableLayout.ts | 5 +---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index 84964dee5c4..a115ff287a7 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -228,12 +228,11 @@ function TableViewBase(props: TableBaseProps, ref: DOMRef = ListLayoutOptions & { columnLayout: TableColumnLayout, - initialCollection: TableCollection, - bodyBorderWidth: number + initialCollection: TableCollection } export class TableLayout extends ListLayout { @@ -40,7 +39,6 @@ export class TableLayout extends ListLayout { uncontrolledColumns: Map>; uncontrolledWidths: Map; resizingColumn: Key | null; - bodyBorderWidth = 0; constructor(options: TableLayoutOptions) { super(options); @@ -52,7 +50,6 @@ export class TableLayout extends ListLayout { this.controlledColumns = controlledColumns; this.uncontrolledColumns = uncontrolledColumns; this.uncontrolledWidths = this.columnLayout.getInitialUncontrolledWidths(uncontrolledColumns); - this.bodyBorderWidth = options.bodyBorderWidth; } protected shouldInvalidateEverything(invalidationContext: InvalidationContext, unknown>): boolean { From b4104f86d4f8f24100f82817f9edc0bad51ea1f3 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 10 Nov 2023 14:34:14 +1100 Subject: [PATCH 07/10] Fix headers not scrolling --- packages/@react-aria/virtualizer/src/VirtualizerItem.tsx | 2 +- packages/@react-stately/layout/src/TableLayout.ts | 1 + packages/@react-stately/virtualizer/src/LayoutInfo.ts | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx index 005642386c4..6300a11a9cf 100644 --- a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx +++ b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx @@ -79,7 +79,7 @@ export function layoutInfoToStyle( WebkitTransition: 'all', WebkitTransitionDuration: 'inherit', transitionDuration: 'inherit', - width: isFullWidth ? '100%' : layoutInfo.rect.width, + width: isFullWidth && layoutInfo.shouldMatchParentWidth ? '100%' : layoutInfo.rect.width, height: layoutInfo.rect.height, opacity: layoutInfo.opacity, zIndex: layoutInfo.zIndex, diff --git a/packages/@react-stately/layout/src/TableLayout.ts b/packages/@react-stately/layout/src/TableLayout.ts index fd15f170d3d..65a4c837af6 100644 --- a/packages/@react-stately/layout/src/TableLayout.ts +++ b/packages/@react-stately/layout/src/TableLayout.ts @@ -152,6 +152,7 @@ export class TableLayout extends ListLayout { buildHeader(): LayoutNode { let rect = new Rect(0, 0, 0, 0); let layoutInfo = new LayoutInfo('header', 'header', rect); + layoutInfo.shouldMatchParentWidth = false; let y = 0; let width = 0; diff --git a/packages/@react-stately/virtualizer/src/LayoutInfo.ts b/packages/@react-stately/virtualizer/src/LayoutInfo.ts index 6983c8d1f3d..951e2b4a3d1 100644 --- a/packages/@react-stately/virtualizer/src/LayoutInfo.ts +++ b/packages/@react-stately/virtualizer/src/LayoutInfo.ts @@ -72,6 +72,13 @@ export class LayoutInfo { */ allowOverflow: boolean; + /** + * Whether the layout info should match the parent's pixel width or be a block/percent + * when an element should be full width. + * @default true + */ + shouldMatchParentWidth: boolean = true; + /** * @param type A string representing the view type. Should be `'item'` for item views. Other types are used by supplementary views. @@ -103,6 +110,7 @@ export class LayoutInfo { res.isSticky = this.isSticky; res.zIndex = this.zIndex; res.allowOverflow = this.allowOverflow; + res.shouldMatchParentWidth = this.shouldMatchParentWidth; return res; } } From 75644e3c2a30636ee793a513a9b722643da2af7c Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 10 Nov 2023 10:35:15 -0800 Subject: [PATCH 08/10] Revert "Fix headers not scrolling" This reverts commit b4104f86d4f8f24100f82817f9edc0bad51ea1f3. --- packages/@react-aria/virtualizer/src/VirtualizerItem.tsx | 2 +- packages/@react-stately/layout/src/TableLayout.ts | 1 - packages/@react-stately/virtualizer/src/LayoutInfo.ts | 8 -------- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx index 6300a11a9cf..005642386c4 100644 --- a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx +++ b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx @@ -79,7 +79,7 @@ export function layoutInfoToStyle( WebkitTransition: 'all', WebkitTransitionDuration: 'inherit', transitionDuration: 'inherit', - width: isFullWidth && layoutInfo.shouldMatchParentWidth ? '100%' : layoutInfo.rect.width, + width: isFullWidth ? '100%' : layoutInfo.rect.width, height: layoutInfo.rect.height, opacity: layoutInfo.opacity, zIndex: layoutInfo.zIndex, diff --git a/packages/@react-stately/layout/src/TableLayout.ts b/packages/@react-stately/layout/src/TableLayout.ts index 65a4c837af6..fd15f170d3d 100644 --- a/packages/@react-stately/layout/src/TableLayout.ts +++ b/packages/@react-stately/layout/src/TableLayout.ts @@ -152,7 +152,6 @@ export class TableLayout extends ListLayout { buildHeader(): LayoutNode { let rect = new Rect(0, 0, 0, 0); let layoutInfo = new LayoutInfo('header', 'header', rect); - layoutInfo.shouldMatchParentWidth = false; let y = 0; let width = 0; diff --git a/packages/@react-stately/virtualizer/src/LayoutInfo.ts b/packages/@react-stately/virtualizer/src/LayoutInfo.ts index 951e2b4a3d1..6983c8d1f3d 100644 --- a/packages/@react-stately/virtualizer/src/LayoutInfo.ts +++ b/packages/@react-stately/virtualizer/src/LayoutInfo.ts @@ -72,13 +72,6 @@ export class LayoutInfo { */ allowOverflow: boolean; - /** - * Whether the layout info should match the parent's pixel width or be a block/percent - * when an element should be full width. - * @default true - */ - shouldMatchParentWidth: boolean = true; - /** * @param type A string representing the view type. Should be `'item'` for item views. Other types are used by supplementary views. @@ -110,7 +103,6 @@ export class LayoutInfo { res.isSticky = this.isSticky; res.zIndex = this.zIndex; res.allowOverflow = this.allowOverflow; - res.shouldMatchParentWidth = this.shouldMatchParentWidth; return res; } } From 86b92f98cfbf43a9561bfadab94774b7beb6c954 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 10 Nov 2023 10:38:30 -0800 Subject: [PATCH 09/10] Add inner div to headers --- .../@react-spectrum/table/src/TableViewBase.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index a115ff287a7..dd7db893515 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -286,7 +286,7 @@ function TableViewBase(props: TableBaseProps, ref: DOMRef, ReactNode>; let renderWrapper = (parent: View, reusableView: View, children: View[], renderChildren: (views: View[]) => ReactElement[]) => { - let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent?.layoutInfo, reusableView.virtualizer.contentSize); + let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent?.layoutInfo, reusableView.virtualizer.visibleRect); if (style.overflow === 'hidden') { style.overflow = 'visible'; // needed to support position: sticky } @@ -620,10 +620,11 @@ function TableVirtualizer(props) { let resizerPosition = layout.getResizerPosition() - 2; - let resizerAtEdge = resizerPosition > Math.max(state.virtualizer.contentSize.width, state.virtualizer.visibleRect.width) - 3; + let visibleRect = state.virtualizer.visibleRect; + let resizerAtEdge = resizerPosition > Math.max(state.virtualizer.contentSize.width, visibleRect.width) - 3; // this should be fine, every movement of the resizer causes a rerender // scrolling can cause it to lag for a moment, but it's always updated - let resizerInVisibleRegion = resizerPosition < state.virtualizer.visibleRect.maxX; + let resizerInVisibleRegion = resizerPosition < visibleRect.maxX; let shouldHardCornerResizeCorner = resizerAtEdge && resizerInVisibleRegion; // minimize re-render caused on Resizers by memoing this @@ -655,7 +656,14 @@ function TableVirtualizer(props) { transition: state.isAnimating ? `none ${state.virtualizer.transitionDuration}ms` : undefined }} ref={headerRef}> - {state.visibleViews[0]} +
+ {state.visibleViews[0]} +
Date: Fri, 10 Nov 2023 17:13:48 -0800 Subject: [PATCH 10/10] try setting overflow-x: hidden instead --- .../virtualizer/src/ScrollView.tsx | 10 +++++----- .../virtualizer/src/VirtualizerItem.tsx | 20 ++++--------------- .../virtualizer/src/useVirtualizerItem.ts | 1 - .../table/src/TableViewBase.tsx | 16 ++++----------- packages/@react-types/sidenav/src/index.d.ts | 1 - 5 files changed, 13 insertions(+), 35 deletions(-) diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index de7f24587f9..348abd8b5a2 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -159,7 +159,10 @@ function ScrollView(props: ScrollViewProps, ref: RefObject) { if (scrollDirection === 'horizontal') { style.overflowX = 'auto'; style.overflowY = 'hidden'; - } else if (scrollDirection === 'vertical') { + } else if (scrollDirection === 'vertical' || contentSize.width === state.width) { + // Set overflow-x: hidden if content size is equal to the width of the scroll view. + // This prevents horizontal scrollbars from flickering during resizing due to resize observer + // firing slower than the frame rate, which may cause an infinite re-render loop. style.overflowY = 'auto'; style.overflowX = 'hidden'; } else { @@ -167,10 +170,7 @@ function ScrollView(props: ScrollViewProps, ref: RefObject) { } innerStyle = { - // Omit width on inner element if it is equal to the width of the outer element. - // Since this is a block element, the browser will automatically make it full width. - // This prevents horizontal scrollbars from flickering when not needed. - width: contentSize.width === state.width ? undefined : contentSize.width, + width: contentSize.width, height: contentSize.height, pointerEvents: isScrolling ? 'none' : 'auto', position: 'relative', diff --git a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx index 005642386c4..066f918ee88 100644 --- a/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx +++ b/packages/@react-aria/virtualizer/src/VirtualizerItem.tsx @@ -11,7 +11,7 @@ */ import {Direction} from '@react-types/shared'; -import {LayoutInfo, Size} from '@react-stately/virtualizer'; +import {LayoutInfo} from '@react-stately/virtualizer'; import React, {CSSProperties, ReactNode, useRef} from 'react'; import {useLocale} from '@react-aria/i18n'; import {useVirtualizerItem, VirtualizerItemOptions} from './useVirtualizerItem'; @@ -33,19 +33,14 @@ export function VirtualizerItem(props: VirtualizerItemProps) { }); return ( -
+
{children}
); } let cache = new WeakMap(); -export function layoutInfoToStyle( - layoutInfo: LayoutInfo, - dir: Direction, - parent?: LayoutInfo | null, - contentSize?: Size -): CSSProperties { +export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent?: LayoutInfo | null): CSSProperties { let xProperty = dir === 'rtl' ? 'right' : 'left'; let cached = cache.get(layoutInfo); if (cached && cached[xProperty] != null) { @@ -61,13 +56,6 @@ export function layoutInfoToStyle( } } - // Determine if the layoutInfo is the full width of its parent. - // In this case, we use 100% as the width rather than a pixel value, - // which avoids issues with flickering scrollbars. - let isFullWidth = parent - ? layoutInfo.rect.x === parent.rect.x && layoutInfo.rect.width === parent.rect.width - : contentSize && layoutInfo.rect.x === 0 && layoutInfo.rect.width === contentSize.width; - let style: CSSProperties = { position: layoutInfo.isSticky ? 'sticky' : 'absolute', // Sticky elements are positioned in normal document flow. Display inline-block so that they don't push other sticky columns onto the following rows. @@ -79,7 +67,7 @@ export function layoutInfoToStyle( WebkitTransition: 'all', WebkitTransitionDuration: 'inherit', transitionDuration: 'inherit', - width: isFullWidth ? '100%' : layoutInfo.rect.width, + width: layoutInfo.rect.width, height: layoutInfo.rect.height, opacity: layoutInfo.opacity, zIndex: layoutInfo.zIndex, diff --git a/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts b/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts index baffdffb71e..3a1539aeb96 100644 --- a/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts +++ b/packages/@react-aria/virtualizer/src/useVirtualizerItem.ts @@ -16,7 +16,6 @@ import {RefObject, useCallback} from 'react'; import {useLayoutEffect} from '@react-aria/utils'; interface IVirtualizer { - contentSize: Size, updateItemSize(key: Key, size: Size): void } diff --git a/packages/@react-spectrum/table/src/TableViewBase.tsx b/packages/@react-spectrum/table/src/TableViewBase.tsx index 934ae0d1120..f748841eedb 100644 --- a/packages/@react-spectrum/table/src/TableViewBase.tsx +++ b/packages/@react-spectrum/table/src/TableViewBase.tsx @@ -286,7 +286,7 @@ function TableViewBase(props: TableBaseProps, ref: DOMRef, ReactNode>; let renderWrapper = (parent: View, reusableView: View, children: View[], renderChildren: (views: View[]) => ReactElement[]) => { - let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent?.layoutInfo, reusableView.virtualizer.visibleRect); + let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent && parent.layoutInfo); if (style.overflow === 'hidden') { style.overflow = 'visible'; // needed to support position: sticky } @@ -620,11 +620,10 @@ function TableVirtualizer(props) { let resizerPosition = layout.getResizerPosition() - 2; - let visibleRect = state.virtualizer.visibleRect; - let resizerAtEdge = resizerPosition > Math.max(state.virtualizer.contentSize.width, visibleRect.width) - 3; + let resizerAtEdge = resizerPosition > Math.max(state.virtualizer.contentSize.width, state.virtualizer.visibleRect.width) - 3; // this should be fine, every movement of the resizer causes a rerender // scrolling can cause it to lag for a moment, but it's always updated - let resizerInVisibleRegion = resizerPosition < visibleRect.maxX; + let resizerInVisibleRegion = resizerPosition < state.virtualizer.visibleRect.maxX; let shouldHardCornerResizeCorner = resizerAtEdge && resizerInVisibleRegion; // minimize re-render caused on Resizers by memoing this @@ -656,14 +655,7 @@ function TableVirtualizer(props) { transition: state.isAnimating ? `none ${state.virtualizer.transitionDuration}ms` : undefined }} ref={headerRef}> -
- {state.visibleViews[0]} -
+ {state.visibleViews[0]}
extends HTMLAttributes } interface IVirtualizer { - contentSize: Size, updateItemSize(key: Key, size: Size): void }