Skip to content

Commit d3c4ffe

Browse files
authored
S2 Table optimizations (#7087)
* S2 Table optimizations * fix merge
1 parent ff56080 commit d3c4ffe

File tree

4 files changed

+60
-74
lines changed

4 files changed

+60
-74
lines changed

packages/@react-spectrum/s2/src/Table.tsx

Lines changed: 38 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ import {
4444
import {centerPadding, getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'};
4545
import {Checkbox} from './Checkbox';
4646
import Chevron from '../ui-icons/Chevron';
47+
import {colorMix, fontRelative, lightDark, size, style} from '../style/spectrum-theme' with {type: 'macro'};
4748
import {ColumnSize} from '@react-types/table';
4849
import {DOMRef, LoadingState, Node} from '@react-types/shared';
49-
import {fontRelative, lightDark, size, style} from '../style/spectrum-theme' with {type: 'macro'};
5050
import {GridNode} from '@react-types/grid';
5151
import {IconContext} from './Icon';
5252
// @ts-ignore
@@ -430,9 +430,8 @@ const cellFocus = {
430430
borderRadius: size(6)
431431
} as const;
432432

433-
function CellFocusRing(props: {isFocusVisible: boolean}) {
434-
let {isFocusVisible} = props;
435-
return <div role="presentation" className={style({...cellFocus, position: 'absolute', inset: 0})({isFocusVisible})} />;
433+
function CellFocusRing() {
434+
return <div role="presentation" className={style({...cellFocus, position: 'absolute', inset: 0})({isFocusVisible: true})} />;
436435
}
437436

438437
const columnStyles = style({
@@ -463,7 +462,10 @@ const columnStyles = style({
463462
default: 'gray-300',
464463
forcedColors: 'ButtonBorder'
465464
},
466-
borderTopWidth: 0,
465+
borderTopWidth: {
466+
default: 0,
467+
isQuiet: 1
468+
},
467469
borderBottomWidth: 1,
468470
borderStartWidth: 0,
469471
borderEndWidth: {
@@ -493,17 +495,18 @@ export interface ColumnProps extends RACColumnProps {
493495
*/
494496
export function Column(props: ColumnProps) {
495497
let {isHeaderRowHovered} = useContext(InternalTableHeaderContext);
498+
let {isQuiet} = useContext(InternalTableContext);
496499
let {allowsResizing, children, align = 'start'} = props;
497500
let isColumnResizable = allowsResizing;
498501

499502
return (
500-
<RACColumn {...props} style={{borderInlineEndColor: 'transparent'}} className={renderProps => columnStyles({...renderProps, isColumnResizable, align})}>
503+
<RACColumn {...props} style={{borderInlineEndColor: 'transparent'}} className={renderProps => columnStyles({...renderProps, isColumnResizable, align, isQuiet})}>
501504
{({allowsSorting, sortDirection, isFocusVisible, sort, startResize, isHovered}) => (
502505
<>
503506
{/* 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
504507
(no need to juggle showing this focus ring if focus is on the menu button and not if it is on the resizer) */}
505508
{/* Separate absolutely positioned element because appyling the ring on the column directly via outline means the ring's required borderRadius will cause the bottom gray border to curve as well */}
506-
<CellFocusRing isFocusVisible={isFocusVisible} />
509+
{isFocusVisible && <CellFocusRing />}
507510
{isColumnResizable ?
508511
(
509512
<ResizableColumnContents allowsSorting={allowsSorting} sortDirection={sortDirection} sort={sort} startResize={startResize} isHovered={isHeaderRowHovered || isHovered} align={align}>
@@ -797,7 +800,8 @@ const selectAllCheckbox = style({
797800

798801
const selectAllCheckboxColumn = style({
799802
padding: 0,
800-
height: '[calc(100% - 1px)]',
803+
height: 'full',
804+
boxSizing: 'border-box',
801805
outlineStyle: 'none',
802806
position: 'relative',
803807
alignContent: 'center',
@@ -806,7 +810,10 @@ const selectAllCheckboxColumn = style({
806810
forcedColors: 'ButtonBorder'
807811
},
808812
borderXWidth: 0,
809-
borderTopWidth: 0,
813+
borderTopWidth: {
814+
default: 0,
815+
isQuiet: 1
816+
},
810817
borderBottomWidth: 1,
811818
borderStyle: 'solid',
812819
backgroundColor: 'gray-75'
@@ -822,6 +829,7 @@ export interface TableHeaderProps<T> extends Omit<RACTableHeaderProps<T>, 'style
822829
export function TableHeader<T extends object>({columns, children}: TableHeaderProps<T>) {
823830
let scale = useScale();
824831
let {selectionBehavior, selectionMode} = useTableOptions();
832+
let {isQuiet} = useContext(InternalTableContext);
825833
let [isHeaderRowHovered, setHeaderRowHovered] = useState(false);
826834

827835
return (
@@ -831,12 +839,12 @@ export function TableHeader<T extends object>({columns, children}: TableHeaderPr
831839
{selectionBehavior === 'toggle' && (
832840
// Also isSticky prop is applied just for the layout, will decide what the RAC api should be later
833841
// @ts-ignore
834-
<RACColumn isSticky width={scale === 'medium' ? 40 : 52} minWidth={scale === 'medium' ? 40 : 52} className={selectAllCheckboxColumn}>
842+
<RACColumn isSticky width={scale === 'medium' ? 40 : 52} minWidth={scale === 'medium' ? 40 : 52} className={selectAllCheckboxColumn({isQuiet})}>
835843
{({isFocusVisible}) => (
836844
<>
837845
{selectionMode === 'single' &&
838846
<>
839-
<CellFocusRing isFocusVisible={isFocusVisible} />
847+
{isFocusVisible && <CellFocusRing />}
840848
<VisuallyHiddenSelectAllLabel />
841849
</>
842850
}
@@ -919,13 +927,8 @@ const checkboxCellStyle = style({
919927
paddingStart: 16,
920928
alignContent: 'center',
921929
height: '[calc(100% - 1px)]',
922-
borderBottomWidth: 0
923-
// TODO: problem with having the checkbox cell itself use the row background color directly instead
924-
// of having a separate white rectangle div base below a div with the row background color set above it as a mask
925-
// is that it doesn't come out as the same color as the other cells because the base below the sticky cell will be the blue of the
926-
// other cells, not the same white base. If I could convert informative-900/10 (and the rest of the rowBackgroundColors) to an equivalent without any opacity
927-
// then this would be possible. Currently waiting request for Spectrum to provide tokens for these equivalent values
928-
// backgroundColor: '--rowBackgroundColor'
930+
borderBottomWidth: 0,
931+
backgroundColor: '--rowBackgroundColor'
929932
});
930933

931934
const cellContent = style({
@@ -952,15 +955,7 @@ const cellContent = style({
952955
margin: {
953956
default: -4,
954957
isSticky: 0
955-
}
956-
});
957-
958-
const cellBackground = style({
959-
position: 'absolute',
960-
top: 0,
961-
bottom: 0,
962-
right: 0,
963-
left: 0,
958+
},
964959
backgroundColor: {
965960
default: 'transparent',
966961
isSticky: '--rowBackgroundColor'
@@ -996,46 +991,30 @@ export function Cell(props: CellProps) {
996991
{...otherProps}>
997992
{({isFocusVisible}) => (
998993
<>
999-
{/*
1000-
// TODO: problem with having the checkbox cell itself use the row background color directly instead
1001-
of having a separate white rectangle div base below a div with the row background color set above it as a mask
1002-
is that it doesn't come out as the same color as the other cells because the base below the sticky cell when other selected cells are scrolled below it will be the blue of the
1003-
other cells, not the same white base. If I could convert informative-900/10 (and the rest of the rowBackgroundColors) to an equivalent without any opacity
1004-
then I could do away with this styling. To reproduce this, comment out the stickyCell gray-25, get rid of the below div and apply backgroundColor: '--rowBackgroundColor' to checkboxCellStyle.
1005-
Having the CellFocusRing here instead of applying a outline on the cell directly also makes it NOT overlap with the border (can be remedied with a -3px outline offset) and applying a border radius to get the curved outline focus ring messes
1006-
with the divider rendered on the cell since those are also borders
1007-
*/}
1008-
<div role="presentation" className={cellBackground({isSticky})} />
1009-
<CellFocusRing isFocusVisible={isFocusVisible} />
994+
{isFocusVisible && <CellFocusRing />}
1010995
<span className={cellContent({...tableVisualOptions, isSticky, align: align || 'start'})}>{children}</span>
1011996
</>
1012997
)}
1013998
</RACCell>
1014999
);
10151000
}
10161001

1002+
// Use color-mix instead of transparency so sticky cells work correctly.
1003+
const selectedBackground = lightDark(colorMix('gray-25', 'informative-900', 10), colorMix('gray-25', 'informative-700', 10));
1004+
const selectedActiveBackground = lightDark(colorMix('gray-25', 'informative-900', 15), colorMix('gray-25', 'informative-700', 15));
10171005
const rowBackgroundColor = {
1018-
default: 'gray-25',
1019-
isFocusVisibleWithin: 'gray-900/7', // table-row-hover-color
1020-
isHovered: 'gray-900/7', // table-row-hover-color
1021-
isPressed: 'gray-900/10', // table-row-hover-color
1022-
isSelected: {
1023-
default: lightDark('informative-900/10', 'informative-700/10'), // table-selected-row-background-color, opacity /10
1024-
isFocusVisibleWithin: lightDark('informative-900/15', 'informative-700/15'), // table-selected-row-background-color, opacity /15
1025-
isHovered: lightDark('informative-900/15', 'informative-700/15'), // table-selected-row-background-color, opacity /15
1026-
isPressed: lightDark('informative-900/15', 'informative-700/15') // table-selected-row-background-color, opacity /15
1006+
default: {
1007+
default: 'gray-25',
1008+
isQuiet: 'transparent'
10271009
},
1028-
isQuiet: {
1029-
default: 'transparent',
1030-
isFocusVisibleWithin: 'gray-900/7', // table-row-hover-color
1031-
isHovered: 'gray-900/7', // table-row-hover-color
1032-
isPressed: 'gray-900/10', // table-row-hover-color
1033-
isSelected: {
1034-
default: lightDark('informative-900/10', 'informative-700/10'), // table-selected-row-background-color, opacity /10
1035-
isFocusVisibleWithin: lightDark('informative-900/15', 'informative-700/15'), // table-selected-row-background-color, opacity /15
1036-
isHovered: lightDark('informative-900/15', 'informative-700/15'), // table-selected-row-background-color, opacity /15
1037-
isPressed: lightDark('informative-900/15', 'informative-700/15') // table-selected-row-background-color, opacity /15
1038-
}
1010+
isFocusVisibleWithin: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color
1011+
isHovered: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color
1012+
isPressed: colorMix('gray-25', 'gray-900', 10), // table-row-hover-color
1013+
isSelected: {
1014+
default: selectedBackground, // table-selected-row-background-color, opacity /10
1015+
isFocusVisibleWithin: selectedActiveBackground, // table-selected-row-background-color, opacity /15
1016+
isHovered: selectedActiveBackground, // table-selected-row-background-color, opacity /15
1017+
isPressed: selectedActiveBackground // table-selected-row-background-color, opacity /15
10391018
},
10401019
forcedColors: {
10411020
default: 'Background'
@@ -1046,7 +1025,7 @@ const row = style<RowRenderProps & S2TableProps>({
10461025
height: 'full',
10471026
position: 'relative',
10481027
boxSizing: 'border-box',
1049-
backgroundColor: rowBackgroundColor,
1028+
backgroundColor: '--rowBackgroundColor',
10501029
'--rowBackgroundColor': {
10511030
type: 'backgroundColor',
10521031
value: rowBackgroundColor

packages/@react-spectrum/s2/style/spectrum-theme.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {Color, createArbitraryProperty, createColorProperty, createMappedProperty, createRenamedProperty, createTheme} from './style-macro';
13+
import {ArbitraryValue} from './types';
14+
import {Color, createArbitraryProperty, createColorProperty, createMappedProperty, createRenamedProperty, createTheme, parseArbitraryValue} from './style-macro';
1415
import {colorScale, colorToken, fontSizeToken, getToken, simpleColorScale, weirdColorToken} from './tokens' with {type: 'macro'};
1516
import type * as CSS from 'csstype';
1617

@@ -87,21 +88,26 @@ export function baseColor(base: keyof typeof color) {
8788
};
8889
}
8990

90-
export function lightDark(light: Color<keyof typeof color>, dark: Color<keyof typeof color>): `[${string}]` {
91-
let [lightColorValue, lightOpacity] = light.split('/');
92-
let [darkColorValue, darkOpacity] = dark.split('/');
93-
let lightColor = color[lightColorValue];
94-
let darkColor = color[darkColorValue];
95-
96-
if (lightOpacity) {
97-
lightColor = `rgb(from ${lightColor} r g b / ${lightOpacity}%)`;
91+
type SpectrumColor = Color<keyof typeof color> | ArbitraryValue;
92+
function parseColor(value: SpectrumColor) {
93+
let arbitrary = parseArbitraryValue(value);
94+
if (arbitrary) {
95+
return arbitrary[0];
9896
}
99-
100-
if (darkOpacity) {
101-
darkColor = `rgb(from ${darkColor} r g b / ${darkOpacity}%)`;
97+
let [colorValue, opacity] = value.split('/');
98+
colorValue = color[colorValue];
99+
if (opacity) {
100+
colorValue = `rgb(from ${colorValue} r g b / ${opacity}%)`;
102101
}
102+
return colorValue;
103+
}
104+
105+
export function lightDark(light: SpectrumColor, dark: SpectrumColor): `[${string}]` {
106+
return `[light-dark(${parseColor(light)}, ${parseColor(dark)})]`;
107+
}
103108

104-
return `[light-dark(${lightColor}, ${darkColor})]`;
109+
export function colorMix(a: SpectrumColor, b: SpectrumColor, percent: number): `[${string}]` {
110+
return `[color-mix(in srgb, ${parseColor(a)}, ${parseColor(b)} ${percent}%)]`;
105111
}
106112

107113
function generateSpacing<K extends number[]>(px: K): {[P in K[number]]: string} {

packages/@react-spectrum/s2/style/style-macro.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ function createValueLookup(values: Array<CSSValue>, atStart = false) {
102102
return map;
103103
}
104104

105-
function parseArbitraryValue(value: any) {
105+
export function parseArbitraryValue(value: any) {
106106
if (typeof value === 'string' && value.startsWith('--')) {
107107
return [`var(${value})`, generateArbitraryValueSelector(value)];
108108
} else if (typeof value === 'string' && value[0] === '[' && value[value.length - 1] === ']') {

packages/@react-spectrum/s2/style/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ type PropertyValue<T> =
5050
? T[number]
5151
: never;
5252

53-
type PropertyValue2<T> = PropertyValue<T> | CustomProperty | `[${string}]`;
53+
export type ArbitraryValue = CustomProperty | `[${string}]`;
54+
type PropertyValue2<T> = PropertyValue<T> | ArbitraryValue;
5455
type Merge<T> = T extends any ? T : never;
5556
type ShorthandValue<T extends Theme, P> =
5657
P extends string[]

0 commit comments

Comments
 (0)