Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@react-spectrum/s2/src/ListView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ function ListSelectionCheckbox({isDisabled}: {isDisabled: boolean}) {
);
}

function isNextSelected(id: Key | undefined, state: ListState<unknown>) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to the utils file since it was being shared by ListView, TreeView, and TableView

export function isNextSelected(id: Key | undefined, state: ListState<unknown>) {
if (id == null || !state) {
return false;
}
Expand Down
171 changes: 141 additions & 30 deletions packages/@react-spectrum/s2/src/TableView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
TableHeaderProps as RACTableHeaderProps,
TableProps as RACTableProps,
ResizableTableContainer,
RowRenderProps,
TableBodyRenderProps,
TableLayout,
TableLoadMoreItem,
Expand All @@ -59,6 +58,7 @@ import {getOwnerDocument} from 'react-aria/private/utils/domHelpers';
import {GridNode} from 'react-stately/private/grid/GridCollection';
import {IconContext} from './Icon';
import intlMessages from '../intl/*.json';
import {isFirstItem, isNextSelected} from './ListView';
import {Key} from '@react-types/shared';
import {LayoutNode} from 'react-stately/private/layout/ListLayout';
import {Menu, MenuItem, MenuSection, MenuTrigger} from './Menu';
Expand Down Expand Up @@ -121,7 +121,8 @@ interface S2TableProps {
/** Handler that is called when more items should be loaded, e.g. while scrolling near the bottom. */
onLoadMore?: () => any,
/** Provides the ActionBar to display when rows are selected in the TableView. */
renderActionBar?: (selectedKeys: 'all' | Set<Key>) => ReactElement
renderActionBar?: (selectedKeys: 'all' | Set<Key>) => ReactElement,
selectionStyle?: 'checkbox' | 'highlight'
}

// TODO: Note that loadMore and loadingState are now on the Table instead of on the TableBody
Expand All @@ -130,7 +131,7 @@ export interface TableViewProps extends Omit<RACTableProps, 'style' | 'className
styles?: StylesPropWithHeight
}

let InternalTableContext = createContext<TableViewProps & {layout?: S2TableLayout<unknown>, setIsInResizeMode?:(val: boolean) => void, isInResizeMode?: boolean, selectionMode?: 'none' | 'single' | 'multiple'}>({});
let InternalTableContext = createContext<TableViewProps & {layout?: S2TableLayout<unknown>, setIsInResizeMode?:(val: boolean) => void, isInResizeMode?: boolean, selectionMode?: 'none' | 'single' | 'multiple', selectionStyle?: 'checkbox' | 'highlight'}>({});

const tableWrapper = style({
minHeight: 0,
Expand Down Expand Up @@ -300,6 +301,7 @@ export const TableView = forwardRef(function TableView(props: TableViewProps, re
onAction,
onLoadMore,
selectionMode = 'none',
selectionStyle = 'checkbox',
...otherProps
} = props;

Expand All @@ -325,8 +327,9 @@ export const TableView = forwardRef(function TableView(props: TableViewProps, re
onLoadMore,
isInResizeMode,
setIsInResizeMode,
selectionMode
}), [isQuiet, density, overflowMode, loadingState, onLoadMore, isInResizeMode, setIsInResizeMode, selectionMode]);
selectionMode,
selectionStyle
}), [isQuiet, density, overflowMode, loadingState, onLoadMore, isInResizeMode, setIsInResizeMode, selectionMode, selectionStyle]);

let scrollRef = useRef<HTMLElement | null>(null);
let isCheckboxSelection = selectionMode === 'multiple' || selectionMode === 'single';
Expand Down Expand Up @@ -371,7 +374,7 @@ export const TableView = forwardRef(function TableView(props: TableViewProps, re
isCheckboxSelection,
isQuiet
})}
selectionBehavior="toggle"
selectionBehavior={selectionStyle === 'highlight' ? 'replace' : 'toggle'}
selectionMode={selectionMode}
onRowAction={onAction}
{...otherProps}
Expand Down Expand Up @@ -487,7 +490,7 @@ const cellFocus = {
} as const;

function CellFocusRing() {
return <div role="presentation" className={style({...cellFocus, position: 'absolute', inset: 0, pointerEvents: 'none'})({isFocusVisible: true})} />;
return <div role="presentation" className={style({...cellFocus, position: 'absolute', inset: 0, pointerEvents: 'none', top: 0, bottom: '[-1px]'})({isFocusVisible: true})} />;
}

const columnStyles = style({
Expand Down Expand Up @@ -552,14 +555,14 @@ export interface ColumnProps extends Omit<RACColumnProps, 'style' | 'className'
* A column within a `<Table>`.
*/
export const Column = forwardRef(function Column(props: ColumnProps, ref: DOMRef<HTMLDivElement>) {
let {isQuiet} = useContext(InternalTableContext);
let {isQuiet, selectionStyle} = useContext(InternalTableContext);
let {allowsResizing, children, align = 'start'} = props;
let domRef = useDOMRef(ref);
let isMenu = allowsResizing || !!props.menuItems;


return (
<RACColumn {...props} ref={domRef} style={{borderInlineEndColor: 'transparent'}} className={renderProps => columnStyles({...renderProps, isMenu, align, isQuiet})}>
<RACColumn {...props} ref={domRef} style={{borderInlineEndColor: 'transparent'}} className={renderProps => columnStyles({...renderProps, isMenu, align, isQuiet, selectionStyle})}>
{({allowsSorting, sortDirection, isFocusVisible, sort, startResize}) => (
<>
{/* Note this is mainly for column's without a dropdown menu. If there is a dropdown menu, the button is styled to have a focus ring for simplicity
Expand Down Expand Up @@ -903,7 +906,7 @@ export interface TableHeaderProps<T> extends Omit<RACTableHeaderProps<T>, 'style
export const TableHeader = /*#__PURE__*/ (forwardRef as forwardRefType)(function TableHeader<T extends object>({columns, dependencies, children}: TableHeaderProps<T>, ref: DOMRef<HTMLDivElement>) {
let scale = useScale();
let {selectionBehavior, selectionMode} = useTableOptions();
let {isQuiet} = useContext(InternalTableContext);
let {isQuiet, selectionStyle} = useContext(InternalTableContext);
let domRef = useDOMRef(ref);

return (
Expand All @@ -912,7 +915,7 @@ export const TableHeader = /*#__PURE__*/ (forwardRef as forwardRefType)(function
ref={domRef}
className={tableHeader}>
{/* Add extra columns for selection. */}
{selectionBehavior === 'toggle' && (
{selectionBehavior === 'toggle' && selectionStyle === 'checkbox' && (
// Also isSticky prop is applied just for the layout, will decide what the RAC api should be later
// @ts-ignore
(<RACColumn isSticky width={scale === 'medium' ? 40 : 52} minWidth={scale === 'medium' ? 40 : 52} className={selectAllCheckboxColumn({isQuiet})}>
Expand Down Expand Up @@ -992,20 +995,38 @@ const cell = style<CellRenderProps & S2TableProps & {isDivider: boolean, isTreeC
fontSize: controlFont(),
alignItems: 'center',
display: 'flex',
borderStyle: {
default: 'none',
isDivider: 'solid'
},
borderEndWidth: {
default: 0,
isDivider: 1
// borderStyle: {
// default: 'none',
// isDivider: 'solid'
// },
// borderEndWidth: {
// default: 0,
// isDivider: 1
// },
boxShadow: {
isDivider: {
default: '[inset -1px 0 0 var(--borderColorGray)]'
// isFirstItem: '[inset -1px 0 0 var(--borderColorGray), inset 0px 1px 0px var(--borderColorBlue)]'
}
},
borderColor: {
default: 'gray-300',
forcedColors: 'ButtonBorder'
}
zIndex: -1
// borderColor: {
// default: 'gray-300',
// forcedColors: 'ButtonBorder'
// }
});

// const divider = raw(
// `&:before {
// content: "";
// position: absolute;
// inset-inline-end: 0;
// top: 0,
// width: 1px;
// height: 100%;
// background-color: var(--borderColorGray)}`
// );

const stickyCell = {
backgroundColor: 'gray-25'
} as const;
Expand Down Expand Up @@ -1065,6 +1086,7 @@ export const Cell = forwardRef(function Cell(props: CellProps, ref: DOMRef<HTMLD
let {children, isSticky, showDivider = false, align, textValue, ...otherProps} = props;
let domRef = useDOMRef(ref);
let tableVisualOptions = useContext(InternalTableContext);
// let {isFirstItem} = useContext(InternalRowContext);
textValue ||= typeof children === 'string' ? children : undefined;

return (
Expand Down Expand Up @@ -1503,11 +1525,23 @@ const rowTextColor = {
forcedColors: 'ButtonText'
} as const;

const row = style<RowRenderProps & S2TableProps>({
const row = style({
height: 'full',
position: 'relative',
boxSizing: 'border-box',
backgroundColor: '--rowBackgroundColor',
backgroundColor: {
default: '--rowBackgroundColor',
selectionStyle: {
highlight: {
default: '--rowBackgroundColor',
isSelected: {
default: colorMix('gray-25', 'blue-900', 10),
isHovered: colorMix('gray-25', 'blue-900', 15),
isPressed: colorMix('gray-25', 'blue-900', 15)
}
}
}
},
'--rowBackgroundColor': {
type: 'backgroundColor',
value: rowBackgroundColor
Expand Down Expand Up @@ -1548,18 +1582,90 @@ const row = style<RowRenderProps & S2TableProps>({
// }
// },
outlineStyle: 'none',
// kinda unfortunate but the border is really only needed for the first item case. the issue is related to the divider
// essentially, the gray box shadow from the divider would appear on top of the blue box shadow but only for the first item
// in order for it to be on the bottom, i used border...couldn't figure out why this was only happening with box shadow
borderTopWidth: 0,
borderBottomWidth: 1,
borderBottomWidth: {
selectionStyle: {
highlight: 0,
checkbox: 1
}
},
borderStartWidth: 0,
borderEndWidth: 0,
borderStyle: 'solid',
borderColor: {
default: 'gray-300',
forcedColors: 'ButtonBorder'
selectionStyle: {
highlight: {
default: 'transparent'
// isFirstItem: {
// default: 'transparent',
// isSelected: 'blue-900'
// }
},
checkbox: 'gray-300'
}
},
'--borderColorGray': {
type: 'borderColor',
value: 'gray-300'
},
'--borderColorBlue': {
type: 'borderColor',
value: 'blue-900'
},
// to avoid affecting the layout, use box shadow instead
// also selected groups still have gray borders in between the items, hard to have two colors with borders because you'll get a diagonal line where the two borders meet
// have more control over how the borders are rendered using box shadow instead
// couldn't add an absolute positioned div because it would mess up the cell count
boxShadow: {
Comment on lines +1612 to +1616
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I entirely understand this part, at the moment each table row renders just the bottom border right so could we just change the color of that border based on if the row is selected and is the last/first row in its selection group?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh is it because of the top most row and if it is selected? Could we use a box shadow for that case only or something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For highlight selection, the borders actually have a width of 0px. The reasoning for the borders having a width of 0px for highlight is for two reasons: 1) layout shifts 2) border rendering in browsers

  1. For ListView or TreeView, we render a border on all four sides and then only conditionally change the width to 0px if the next or previous item was selected. This didn't cause any layout shifts because this was done to an absolute position div. However, we can't conditionally change the border width in TableView without causing a layout shift because we can't add an absolute position div. As a result, we use box-shadows to avoid this issue.

  2. In the case for a selected item (assuming the next item is also selected), the left and right borders will be blue while the bottom border will be gray. Using borders, the blue and gray borders meet at a diagonal edge due to how browsers render border joins. See codepen. This wasn't an issue in ListView or TreeView because selected items don't have any borders between them.

Copy link
Copy Markdown
Member Author

@yihuiliao yihuiliao Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The border for the first row is handled slightly differently due to issues with the CSS paint order. Initially, I also just used a box shadow to render the top border of the first item if it was selected, but if a cell has a divider, the divider gets rendered on top of it so you have a gray line on top of the blue line. So in order to fix that, I had to use a pseudo-element to basically force it on top of the divider. I tried isolation and z-indexes which didn't work

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm gotcha, I actually used a outline here (kinda like the commented out code) for the "on" drop indicators, could we do the same here instead then?

outlineStyle: {
default: 'none',
isDropTarget: 'solid',
forcedColors: {
isFocusVisible: 'solid'
}
},
outlineWidth: {
isDropTarget: 2,
forcedColors: {
isFocusVisible: 2
}
},
outlineOffset: {
isDropTarget: -2,
forcedColors: {
isFocusVisible: -2
}
},
outlineColor: {
isDropTarget: 'blue-800',
forcedColors: 'Highlight'
},

If not, I'm happy to switch my implementation to use box shadows completely, but whichever way we choose should be used for both "on" drop and highlight selection for sure

selectionStyle: {
highlight: {
default: '[inset 0px -1px 0px var(--borderColorGray)]',
isNextSelected: '[inset 0px -1px 0px var(--borderColorBlue)]',
isSelected: {
default: '[inset 0px -1px 0px var(--borderColorBlue), inset 1px 0px 0px var(--borderColorBlue), inset -1px 0px var(--borderColorBlue)]',
isNextSelected: '[inset 1px 0px 0px var(--borderColorBlue), inset -1px 0px var(--borderColorBlue), inset 0px -1px 0px var(--borderColorGray)]',
isFirstItem: {
default: '[inset 0px 1px 0px var(--borderColorBlue), inset 0px -1px 0px var(--borderColorBlue), inset 1px 0px 0px var(--borderColorBlue), inset -1px 0px var(--borderColorBlue)]',
isNextSelected: '[inset 1px 0px 0px var(--borderColorBlue), inset -1px 0px 0px var(--borderColorBlue), inset 0px -1px 0px var(--borderColorGray)]'
}
}
}
}
},
'--focusIndicatorHeight': {
type: 'top',
value: {
default: 'calc(self(height) - 1px)'
}
},
isolation: 'isolate',
zIndex: 3,
forcedColorAdjust: 'none'
});

const border = css(
`&:after {
content: "";
width: 100%;
height: 100%;
top: 0;
inset-inline-start: 0;
z-index: 3;
position: absolute;
box-sizing: border-box;
border-top-width: 1px;
border-bottom-width: 0px;
border-inline-start-width: 0px;
border-inline-end-width: 0px;
border-style: solid;
border-color: var(--borderColorBlue);
}
`
);

const selectionCheckbox = style({
visibility: {
default: 'visible',
Expand All @@ -1574,7 +1680,7 @@ export interface RowProps<T> extends Pick<RACRowProps<T>, 'id' | 'columns' | 'is
*/
export const Row = /*#__PURE__*/ (forwardRef as forwardRefType)(function Row<T extends object>({id, columns, children, dependencies = [], ...otherProps}: RowProps<T>, ref: DOMRef<HTMLDivElement>) {
let {selectionBehavior, selectionMode} = useTableOptions();
let tableVisualOptions = useContext(InternalTableContext);
let {selectionStyle, ...tableVisualOptions} = useContext(InternalTableContext);
let domRef = useDOMRef(ref);

return (
Expand All @@ -1585,8 +1691,13 @@ export const Row = /*#__PURE__*/ (forwardRef as forwardRefType)(function Row<T e
dependencies={[...dependencies, columns]}
className={renderProps => row({
...renderProps,
...tableVisualOptions
}) + (renderProps.isFocusVisible ? ' ' + css('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)') : '')}
...tableVisualOptions,
selectionStyle,
isNextSelected: isNextSelected(id, renderProps.state),
isFirstItem: isFirstItem(id, renderProps.state)
}) + (renderProps.isFocusVisible ? ' ' + css('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: var(--focusIndicatorHeight); margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)') : '')
+ (isFirstItem(id, renderProps.state) && renderProps.isSelected && selectionStyle === 'highlight' ? ' ' + border : '')
}
{...otherProps}>
{selectionMode !== 'none' && selectionBehavior === 'toggle' && (
// Not sure what we want to do with this className, in Cell it currently overrides the className that would have been applied.
Expand Down
13 changes: 13 additions & 0 deletions packages/@react-spectrum/s2/stories/TableView.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ export const Example: StoryObj<typeof StaticTable> = {
}
};

export const Highlight: StoryObj<typeof StaticTable> = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need it added to docs

do we need a special one for storybook? or can the controls cover this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added it only because it was easier to have an already made example of it rather than having to use the controls. we also do the same in ListView but i can remove it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's aim for less stories <3
We'll cover the initial rendering with chromatic

render: StaticTable,
args: {
selectionMode: 'multiple',
selectionStyle: 'highlight',
onResize: undefined,
onResizeStart: undefined,
onResizeEnd: undefined,
onLoadMore: undefined
}
};


export const DisabledRows: StoryObj<typeof StaticTable> = {
...Example,
args: {
Expand Down
4 changes: 3 additions & 1 deletion packages/react-aria-components/src/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,8 @@ export interface RowRenderProps extends ItemRenderProps {
* What level the row has within the table.
* @selector [data-level]
*/
level: number
level: number,
state: TableState<unknown>
}

export interface RowProps<T> extends StyleRenderProps<RowRenderProps, 'tr' | 'div'>, LinkDOMProps, HoverEvents, PressEvents, Omit<GlobalDOMAttributes<HTMLTableRowElement>, 'onClick'> {
Expand Down Expand Up @@ -1427,6 +1428,7 @@ export const Row = /*#__PURE__*/ createBranchComponent(
},
values: {
...states,
state,
isHovered,
isFocused,
isFocusVisible,
Expand Down
Loading