Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Chromatic/testing found bugs #7819

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

fix: Chromatic/testing found bugs #7819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0b1048a
fix: Chromatic found bugs
snowystinger Feb 24, 2025
e2f694a
fix dead docs link
snowystinger Feb 24, 2025
fb938ea
Merge branch 'main' into chromatic-fixes
snowystinger Feb 24, 2025
861ac2f
TableView center wrapper account for 40px margin
snowystinger Feb 24, 2025
70e66cf
Merge branch 'chromatic-fixes' of github.com:adobe/react-spectrum int…
snowystinger Feb 24, 2025
d0c8016
fix treeview virtualization and empty state focus
snowystinger Feb 25, 2025
c244d28
fix RAC Toast docs direction
snowystinger Feb 25, 2025
88a8f07
fix missing v3 unsafe classname, overflow auto, dynamic rendering
snowystinger Feb 25, 2025
e70d4e4
fix v3 focus ring empty collection
snowystinger Feb 25, 2025
d65bde5
adjust centering again
snowystinger Feb 25, 2025
3413226
adjust math for centering
snowystinger Feb 25, 2025
232e598
use height of headers instead
snowystinger Feb 25, 2025
babd4f8
revert margin change
snowystinger Feb 25, 2025
57821f9
fix double export of layouts
snowystinger Feb 25, 2025
e6ecaca
remove unused prop, docs, and fix typo
snowystinger Feb 26, 2025
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
9 changes: 0 additions & 9 deletions packages/@react-aria/virtualizer/src/VirtualizerItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,6 @@ export function layoutInfoToStyle(layoutInfo: LayoutInfo, dir: Direction, parent
...rectStyles
};

if (layoutInfo.isSticky && !parent?.allowOverflow) {
Copy link
Member Author

Choose a reason for hiding this comment

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

haven't found any place that this code was affecting other than to make our S2 Tables empty and spinners not centered

if (style.top) {
style.marginTop = style.top;
}
if (style[xProperty]) {
style[dir === 'rtl' ? 'marginRight' : 'marginLeft'] = style[xProperty];
}
}

cache.set(layoutInfo, style);
return style;
}
2 changes: 1 addition & 1 deletion packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ const DynamicTreeItem = (props) => {
let {childItems, name, icon} = props;
return (
<>
<TreeViewItem id={props.id} childItems={childItems} textValue={name} href={props.href}>
<TreeViewItem id={props.id} textValue={name} href={props.href}>
<TreeViewItemContent>
<Text>{name}</Text>
{icon}
Expand Down
1 change: 1 addition & 0 deletions packages/@react-spectrum/s2/src/TableView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ export class S2TableLayout<T> extends TableLayout<T> {
return layoutNode;
}

// y is the height of the headers
protected buildBody(y: number): LayoutNode {
let layoutNode = super.buildBody(y);
let {children, layoutInfo} = layoutNode;
Expand Down
20 changes: 9 additions & 11 deletions packages/@react-spectrum/s2/src/TreeView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
import {centerBaseline} from './CenterBaseline';
import {Checkbox} from './Checkbox';
import Chevron from '../ui-icons/Chevron';
import {colorMix, fontRelative, lightDark, style} from '../style' with {type: 'macro'};
import {colorMix, focusRing, fontRelative, lightDark, style} from '../style' with {type: 'macro'};
import {DOMRef, Key} from '@react-types/shared';
import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'};
import {IconContext} from './Icon';
Expand All @@ -54,11 +54,9 @@ export interface TreeViewProps extends Omit<RACTreeProps<any>, 'style' | 'classN
styles?: StylesPropWithHeight
}

export interface TreeViewItemProps<T extends object = object> extends Omit<RACTreeItemProps, 'className' | 'style'> {
export interface TreeViewItemProps extends Omit<RACTreeItemProps, 'className' | 'style'> {
/** Whether this item has children, even if not loaded yet. */
hasChildItems?: boolean,
/** A list of child tree item objects used when dynamically rendering the tree item children. */
childItems?: Iterable<T>
hasChildItems?: boolean
}

interface TreeRendererContextValue {
Expand All @@ -73,10 +71,13 @@ let InternalTreeContext = createContext<{isDetached?: boolean, isEmphasized?: bo
// keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't
// scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview
const tree = style({
...focusRing(),
outlineOffset: -2, // make certain we are visible inside overflow hidden containers
userSelect: 'none',
minHeight: 0,
minWidth: 0,
width: 'full',
height: 'full',
overflow: 'auto',
boxSizing: 'border-box',
justifyContent: {
Expand All @@ -85,9 +86,6 @@ const tree = style({
alignItems: {
isEmpty: 'center'
},
height: {
isEmpty: 'full'
},
'--indent': {
type: 'width',
value: 16
Expand All @@ -110,13 +108,13 @@ function TreeView(props: TreeViewProps, ref: DOMRef<HTMLDivElement>) {
layout={ListLayout}
layoutOptions={{
rowHeight: scale === 'large' ? 50 : 40,
gap: isDetached ? 4 : 0
gap: isDetached ? 2 : 0
}}>
<TreeRendererContext.Provider value={{renderer}}>
<InternalTreeContext.Provider value={{isDetached, isEmphasized}}>
<Tree
{...props}
className={({isEmpty}) => tree({isEmpty, isDetached}, props.styles)}
className={renderProps => tree({isDetached, ...renderProps}, props.styles)}
selectionBehavior="toggle"
ref={domRef}>
{props.children}
Expand Down Expand Up @@ -295,7 +293,7 @@ const treeRowFocusIndicator = raw(`
}`
);

export const TreeViewItem = <T extends object>(props: TreeViewItemProps<T>) => {
export const TreeViewItem = (props: TreeViewItemProps) => {
let {
href
} = props;
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/s2/stories/TreeView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ const DynamicTreeItem = (props) => {
let {childItems, name, icon} = props;
return (
<>
<TreeViewItem id={props.id} childItems={childItems} textValue={name} href={props.href}>
<TreeViewItem id={props.id} textValue={name} href={props.href}>
<TreeViewItemContent>
<Text>{name}</Text>
{icon}
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/style-macro-s1/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@
* governing permissions and limitations under the License.
*/

export {style, baseColor, lightDark} from './spectrum-theme';
export {style, baseColor, lightDark, focusRing} from './spectrum-theme';
export {raw, keyframes} from './style-macro';
10 changes: 10 additions & 0 deletions packages/@react-spectrum/style-macro-s1/src/spectrum-theme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,16 @@ let durationProperty = createArbitraryProperty((value: number | string, property

const colorWithAlpha = createColorProperty(color);

export const focusRing = () => ({
outlineStyle: {
default: 'none',
isFocusVisible: 'solid'
},
outlineColor: 'focus-ring',
outlineWidth: 2,
outlineOffset: 2
} as const);

export const style = createTheme({
properties: {
// colors
Expand Down
86 changes: 49 additions & 37 deletions packages/@react-spectrum/tree/docs/TreeView.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ let items: MyItem[] = [
const DynamicTreeItem = (props) => {
return (
<>
<TreeViewItem id={props.id} childItems={props.childItems} textValue={props.name}>
<TreeViewItem id={props.id} textValue={props.name}>
<TreeViewItemContent>
<Text>{props.name}</Text>
{props.icon}
Expand Down Expand Up @@ -419,49 +419,61 @@ The `<TreeViewItem>` component works with frameworks and client side routers lik

```tsx example
<TreeView aria-label="Example tree with action groups" height="size-3000" maxWidth="size-6000" items={items}>
{(item: MyItem) => (
<TreeViewItem childItems={item.childItems} textValue={item.name}>
<TreeViewItemContent>
<Text>{item.name}</Text>
{item.icon}
<ActionGroup onAction={(key) => alert(`Item: ${item.id}, Action: ${key}`)}>
<Item key="edit" textValue="Edit">
<Edit />
<Text>Edit</Text>
</Item>
<Item key="delete" textValue="Delete">
<Delete />
<Text>Delete</Text>
</Item>
</ActionGroup>
</TreeViewItemContent>
</TreeViewItem>
)}
{function renderItem(item: MyItem) {
return (
<TreeViewItem textValue={item.name}>
<TreeViewItemContent>
<Text>{item.name}</Text>
{item.icon}
<ActionGroup onAction={(key) => alert(`Item: ${item.id}, Action: ${key}`)}>
<Item key="edit" textValue="Edit">
<Edit />
<Text>Edit</Text>
</Item>
<Item key="delete" textValue="Delete">
<Delete />
<Text>Delete</Text>
</Item>
</ActionGroup>
</TreeViewItemContent>
<Collection items={item.childItems}>
{renderItem}
</Collection>
</TreeViewItem>
)
}}
</TreeView>
```

### With action menus

```tsx example
import {Collection} from 'react-aria-components';

<TreeView aria-label="Example tree with action menus" height="size-3000" maxWidth="size-6000" items={items}>
{(item: MyItem) => (
<TreeViewItem childItems={item.childItems} textValue={item.name}>
<TreeViewItemContent>
<Text>{item.name}</Text>
{item.icon}
<ActionMenu onAction={(key) => alert(`Item: ${item.id}, Action: ${key}`)}>
<Item key="edit" textValue="Edit">
<Edit />
<Text>Edit</Text>
</Item>
<Item key="delete" textValue="Delete">
<Delete />
<Text>Delete</Text>
</Item>
</ActionMenu>
</TreeViewItemContent>
</TreeViewItem>
)}
{function renderItem(item: MyItem) {
return (
<TreeViewItem textValue={item.name}>
<TreeViewItemContent>
<Text>{item.name}</Text>
{item.icon}
<ActionMenu onAction={(key) => alert(`Item: ${item.id}, Action: ${key}`)}>
<Item key="edit" textValue="Edit">
<Edit />
<Text>Edit</Text>
</Item>
<Item key="delete" textValue="Delete">
<Delete />
<Text>Delete</Text>
</Item>
</ActionMenu>
</TreeViewItemContent>
<Collection items={item.childItems}>
{renderItem}
</Collection>
</TreeViewItem>
)
}}
</TreeView>
```

Expand Down
27 changes: 12 additions & 15 deletions packages/@react-spectrum/tree/src/TreeView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import {Checkbox} from '@react-spectrum/checkbox';
import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium';
import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium';
import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared';
import {focusRing, style} from '@react-spectrum/style-macro-s1' with {type: 'macro'};
import {isAndroid} from '@react-aria/utils';
import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useRef} from 'react';
import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils';
import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'};
import {useButton} from '@react-aria/button';
import {useLocale} from '@react-aria/i18n';

Expand All @@ -48,11 +48,9 @@ export interface SpectrumTreeViewProps<T> extends Omit<AriaTreeProps<T>, 'childr
children?: ReactNode | ((item: T) => ReactNode)
}

export interface SpectrumTreeViewItemProps<T extends object = object> extends Omit<TreeItemProps, 'className' | 'style' | 'value' | 'onHoverStart' | 'onHoverEnd' | 'onHoverChange'> {
export interface SpectrumTreeViewItemProps extends Omit<TreeItemProps, 'className' | 'style' | 'value' | 'onHoverStart' | 'onHoverEnd' | 'onHoverChange'> {
/** Rendered contents of the tree item or child items. */
children: ReactNode,
/** A list of child tree item objects used when dynamically rendering the tree item children. */
childItems?: Iterable<T>
children: ReactNode
}

interface TreeRendererContextValue {
Expand All @@ -66,6 +64,10 @@ const TreeRendererContext = createContext<TreeRendererContextValue>({});
// keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't
// scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview
const tree = style<Pick<TreeRenderProps, 'isEmpty'>>({
...focusRing(),
outlineOffset: '[-2px]', // make certain we are visible inside overflow hidden containers
height: 'full',
width: 'full',
borderWidth: 2,
boxSizing: 'border-box',
borderXWidth: 0,
Expand All @@ -80,22 +82,17 @@ const tree = style<Pick<TreeRenderProps, 'isEmpty'>>({
alignItems: {
isEmpty: 'center'
},
width: {
isEmpty: 'full'
},
height: {
isEmpty: 'full'
},
display: {
isEmpty: 'flex'
}
},
overflow: 'auto'
});

/**
* A tree view provides users with a way to navigate nested hierarchical information.
*/
export const TreeView = React.forwardRef(function TreeView<T extends object>(props: SpectrumTreeViewProps<T>, ref: DOMRef<HTMLDivElement>) {
let {children, selectionStyle} = props;
let {children, selectionStyle, UNSAFE_className} = props;

let renderer;
if (typeof children === 'function') {
Expand All @@ -108,7 +105,7 @@ export const TreeView = React.forwardRef(function TreeView<T extends object>(pro

return (
<TreeRendererContext.Provider value={{renderer}}>
<Tree {...props} {...styleProps} className={({isEmpty}) => tree({isEmpty})} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}>
<Tree {...props} {...styleProps} className={renderProps => UNSAFE_className + tree(renderProps)} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}>
{props.children}
</Tree>
</TreeRendererContext.Provider>
Expand Down Expand Up @@ -228,7 +225,7 @@ const treeRowOutline = style({
}
});

export const TreeViewItem = <T extends object>(props: SpectrumTreeViewItemProps<T>) => {
export const TreeViewItem = (props: SpectrumTreeViewItemProps) => {
let {
href
} = props;
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/tree/stories/TreeView.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ const DynamicTreeItem = (props) => {
let {childItems, name, icon} = props;
return (
<>
<TreeViewItem id={props.id} childItems={childItems} textValue={name} href={props.href}>
<TreeViewItem id={props.id} textValue={name} href={props.href}>
<TreeViewItemContent>
<Text>{name}</Text>
{icon}
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/tree/test/TreeView.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ const DynamicTreeItem = (props) => {
let {childItems, name} = props;
return (
<>
<TreeViewItem id={props.id} childItems={childItems} textValue={name} href={props.href}>
<TreeViewItem id={props.id} textValue={name} href={props.href}>
<TreeViewItemContent>
<Text>{name}</Text>
<ActionGroup>
Expand Down
Loading