Skip to content

fix: Tabs from testing #7463

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

Merged
snowystinger merged 6 commits into from
Dec 4, 2024
Merged

fix: Tabs from testing #7463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cd94245
fix: Tabs from testing
snowystinger Dec 4, 2024
ff8e5c9
fix horizontal spacing for icon only
snowystinger Dec 4, 2024
09a7d5e
more spacing fixes
snowystinger Dec 4, 2024
aff0815
make mobile testing easier
snowystinger Dec 4, 2024
d092f95
rename prop isIconOnly
snowystinger Dec 4, 2024
dcb79e9
Merge branch 'main' into tab-fixes-from-testing
snowystinger Dec 4, 2024
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
10 changes: 5 additions & 5 deletions packages/@react-spectrum/s2/chromatic/Tabs.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default meta;
export const Example = (args: any) => (
<Tabs {...args} styles={style({width: 450, height: 256})}>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR"><Edit /><Text>Founding of Rome</Text></Tab>
<Tab id="FoR">Founding of Rome</Tab>
<Tab id="MaR">Monarchy and Republic</Tab>
<Tab id="Emp">Empire</Tab>
</TabList>
Expand All @@ -57,9 +57,9 @@ export const Example = (args: any) => (
export const Disabled = (args: any) => (
<Tabs {...args} styles={style({width: 450, height: 144})} disabledKeys={['FoR', 'MaR', 'Emp']}>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR"><Edit /><Text>Founding of Rome</Text></Tab>
<Tab id="MaR">Monarchy and Republic</Tab>
<Tab id="Emp">Empire</Tab>
<Tab id="FoR" aria-label="Edit"><Edit /><Text>Edit</Text></Tab>
Copy link
Copy Markdown
Collaborator

@ktabors ktabors Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should the keys change to match the new values? Why are we disabling all the keys?

Copy link
Copy Markdown
Member Author

@snowystinger snowystinger Dec 4, 2024

Choose a reason for hiding this comment

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

a bit of knowledge about the internal workings that is being tested to make sure that disabling all the keys individually works the same
this is a chromatic story, not fussed about the keys

<Tab id="MaR" aria-label="Notifications"><Bell /><Text>Notifications</Text></Tab>
<Tab id="Emp" aria-label="Likes"><Heart /><Text>Likes</Text></Tab>
</TabList>
<TabPanel id="FoR">
Arma virumque cano, Troiae qui primus ab oris.
Expand All @@ -74,7 +74,7 @@ export const Disabled = (args: any) => (
);

export const Icons = (args: any) => (
<Tabs {...args} styles={style({width: 208, height: 144})} iconOnly>
<Tabs {...args} styles={style({width: 208, height: 144})} isIconOnly>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR" aria-label="Edit"><Edit /><Text>Edit</Text></Tab>
<Tab id="MaR" aria-label="Notifications"><Bell /><Text>Notifications</Text></Tab>
Expand Down
37 changes: 24 additions & 13 deletions packages/@react-spectrum/s2/src/Tabs.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 {Collection, DOMRef, DOMRefValue, FocusableRef, FocusableRefValue, Key, Node, Orientation, RefObject} from '@react-types/shared';
import {createContext, forwardRef, Fragment, ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
import {focusRing, style} from '../style' with {type: 'macro'};
import {focusRing, size, style} from '../style' with {type: 'macro'};
import {getAllowedOverrides, StyleProps, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'};
import {IconContext} from './Icon';
// @ts-ignore
Expand All @@ -55,7 +55,7 @@ export interface TabsProps extends Omit<AriaTabsProps, 'className' | 'style' | '
/**
* If the tabs should only display icons and no text.
*/
iconOnly?: boolean
isIconOnly?: boolean
}

export interface TabProps extends Omit<AriaTabProps, 'children' | 'style' | 'className'>, StyleProps {
Expand Down Expand Up @@ -96,7 +96,7 @@ export const Tabs = forwardRef(function Tabs(props: TabsProps, ref: DOMRef<HTMLD
isDisabled,
disabledKeys,
orientation = 'horizontal',
iconOnly = false
isIconOnly = false
} = props;
let domRef = useDOMRef(ref);
let [value, setValue] = useControlledState(props.selectedKey, props.defaultSelectedKey ?? null!, props.onSelectionChange);
Expand All @@ -112,7 +112,7 @@ export const Tabs = forwardRef(function Tabs(props: TabsProps, ref: DOMRef<HTMLD
disabledKeys,
selectedKey: value,
onSelectionChange: setValue,
iconOnly,
isIconOnly,
onFocus: () => pickerRef.current?.focus(),
pickerRef
}]
Expand Down Expand Up @@ -170,7 +170,7 @@ const tablist = style({
});

export function TabList<T extends object>(props: TabListProps<T>) {
let {density, isDisabled, disabledKeys, orientation, iconOnly, onFocus} = useContext(InternalTabsContext) ?? {};
let {density, isDisabled, disabledKeys, orientation, isIconOnly, onFocus} = useContext(InternalTabsContext) ?? {};
let {showItems} = useContext(CollapseContext) ?? {};
let state = useContext(TabListStateContext);
let [selectedTab, setSelectedTab] = useState<HTMLElement | undefined>(undefined);
Expand Down Expand Up @@ -208,7 +208,7 @@ export function TabList<T extends object>(props: TabListProps<T>) {
<RACTabList
{...props}
ref={tablistRef}
className={renderProps => tablist({...renderProps, isIconOnly: iconOnly, density})} />
className={renderProps => tablist({...renderProps, isIconOnly, density})} />
{orientation === 'horizontal' &&
<TabLine showItems={showItems} disabledKeys={disabledKeys} isDisabled={isDisabled} selectedTab={selectedTab} orientation={orientation} density={density} />}
</div>
Expand Down Expand Up @@ -255,7 +255,7 @@ const selectedIndicator = style({
transitionTimingFunction: 'in-out'
});

function TabLine(props: TabLineProps) {
function TabLine(props: TabLineProps & {isIconOnly?: boolean}) {
let {
disabledKeys,
isDisabled: isTabsDisabled,
Expand Down Expand Up @@ -301,7 +301,14 @@ function TabLine(props: TabLineProps) {

useLayoutEffect(() => {
onResize();
}, [onResize, state?.selectedItem?.key, direction, orientation, density]);
}, [onResize, state?.selectedItem?.key, density]);

let ref = useRef<HTMLElement | undefined>(selectedTab);
// assign ref before the useResizeObserver useEffect runs
useLayoutEffect(() => {
ref.current = selectedTab;
});
useResizeObserver({ref, onResize});

return (
<div style={{...style}} className={selectedIndicator({isDisabled, orientation})} />
Expand Down Expand Up @@ -333,7 +340,10 @@ const tab = style({
position: 'relative',
cursor: 'default',
flexShrink: 0,
transition: 'default'
transition: 'default',
paddingX: {
isIconOnly: size(6)
}
}, getAllowedOverrides());

const icon = style({
Expand All @@ -346,15 +356,15 @@ const icon = style({
});

export function Tab(props: TabProps) {
let {density, iconOnly} = useContext(InternalTabsContext) ?? {};
let {density, isIconOnly} = useContext(InternalTabsContext) ?? {};

return (
<RACTab
{...props}
// @ts-ignore
originalProps={props}
style={props.UNSAFE_style}
className={renderProps => (props.UNSAFE_className || '') + tab({...renderProps, density}, props.styles)}>
className={renderProps => (props.UNSAFE_className || '') + tab({...renderProps, density, isIconOnly}, props.styles)}>
{({
// @ts-ignore
isMenu
Expand All @@ -372,7 +382,7 @@ export function Tab(props: TabProps) {
display: {
isIconOnly: 'none'
}
})({isIconOnly: iconOnly})
})({isIconOnly})
}],
[IconContext, {
render: centerBaseline({slot: 'icon', styles: style({order: 0})}),
Expand Down Expand Up @@ -469,7 +479,7 @@ let HiddenTabs = function (props: {
let TabsMenu = (props: {items: Array<Node<any>>, onSelectionChange: TabsProps['onSelectionChange']}) => {
let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2');
let {items} = props;
let {density, onSelectionChange, selectedKey, isDisabled, disabledKeys, pickerRef} = useContext(InternalTabsContext);
let {density, onSelectionChange, selectedKey, isDisabled, disabledKeys, pickerRef, isIconOnly} = useContext(InternalTabsContext);
let state = useContext(TabListStateContext);
let allKeysDisabled = useMemo(() => {
return isAllTabsDisabled(state?.collection, disabledKeys ? new Set(disabledKeys) : new Set());
Expand All @@ -491,6 +501,7 @@ let TabsMenu = (props: {items: Array<Node<any>>, onSelectionChange: TabsProps['o
ref={pickerRef ? pickerRef : undefined}
isDisabled={isDisabled || allKeysDisabled}
density={density!}
isIconOnly={isIconOnly}
items={items}
disabledKeys={disabledKeys}
selectedKey={selectedKey}
Expand Down
29 changes: 22 additions & 7 deletions packages/@react-spectrum/s2/src/TabsPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ import {
checkmark,
description,
icon,
iconCenterWrapper,
label,
menuitem,
sectionHeader,
sectionHeading
} from './Menu';
import CheckmarkIcon from '../ui-icons/Checkmark';
import ChevronIcon from '../ui-icons/Chevron';
import {edgeToText, focusRing, style} from '../style' with {type: 'macro'};
import {edgeToText, focusRing, size, style} from '../style' with {type: 'macro'};
import {fieldInput, StyleProps} from './style-utils' with {type: 'macro'};
import {
FieldLabel
Expand Down Expand Up @@ -86,7 +85,11 @@ export interface PickerProps<T extends object> extends
/** Width of the menu. By default, matches width of the trigger. Note that the minimum width of the dropdown is always equal to the trigger's width. */
menuWidth?: number,
/** Density of the tabs, affects the height of the picker. */
density: 'compact' | 'regular'
density: 'compact' | 'regular',
/**
* If the tab picker should only display icon and no text for the button label.
*/
isIconOnly?: boolean
}

export const PickerContext = createContext<ContextValue<Partial<PickerProps<any>>, FocusableRefValue<HTMLButtonElement>>>(null);
Expand Down Expand Up @@ -155,6 +158,14 @@ const iconStyles = style({
}
});

const iconCenterWrapper = style({
display: 'flex',
gridArea: 'icon',
paddingStart: {
isIconOnly: size(6)
}
});

let InsideSelectValueContext = createContext(false);

function Picker<T extends object>(props: PickerProps<T>, ref: FocusableRef<HTMLButtonElement>) {
Expand All @@ -170,6 +181,7 @@ function Picker<T extends object>(props: PickerProps<T>, ref: FocusableRef<HTMLB
items,
placeholder = stringFormatter.format('picker.placeholder'),
density,
isIconOnly,
...pickerProps
} = props;
let isQuiet = true;
Expand Down Expand Up @@ -205,7 +217,7 @@ function Picker<T extends object>(props: PickerProps<T>, ref: FocusableRef<HTMLB
[IconContext, {
slots: {
icon: {
render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}),
render: centerBaseline({slot: 'icon', styles: iconCenterWrapper({isIconOnly})}),
styles: icon
}
}
Expand All @@ -214,10 +226,13 @@ function Picker<T extends object>(props: PickerProps<T>, ref: FocusableRef<HTMLB
slots: {
// Default slot is useful when converting other collections to PickerItems.
[DEFAULT_SLOT]: {styles: style({
display: 'block',
display: {
default: 'block',
isIconOnly: 'none'
},
flexGrow: 1,
truncate: true
})}
})({isIconOnly})}
}
}],
[InsideSelectValueContext, true]
Expand Down Expand Up @@ -291,7 +306,7 @@ export function PickerItem(props: PickerItemProps) {
<DefaultProvider
context={IconContext}
value={{slots: {
icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper}), styles: icon}
icon: {render: centerBaseline({slot: 'icon', styles: iconCenterWrapper({})}), styles: icon}
}}}>
<DefaultProvider
context={TextContext}
Expand Down
20 changes: 12 additions & 8 deletions packages/@react-spectrum/s2/stories/Tabs.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ const meta: Meta<typeof Tabs> = {
export default meta;

export const Example = (args: any) => (
<div className={style({width: 700, height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<div className={style({width: 700, maxWidth: '[calc(100vw - 60px)]', height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<Tabs {...args} styles={style({width: 'full'})}>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR"><Edit /><Text>Founding of Rome</Text></Tab>
<Tab id="FoR">Founding of Rome</Tab>
<Tab id="MaR">Monarchy and Republic</Tab>
<Tab id="Emp">Empire</Tab>
</TabList>
Expand All @@ -58,10 +58,10 @@ export const Example = (args: any) => (
);

export const Disabled = (args: any) => (
<div className={style({width: 700, height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<div className={style({width: 700, maxWidth: '[calc(100vw - 60px)]', height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
Comment thread
snowystinger marked this conversation as resolved.
<Tabs {...args} styles={style({width: 'full'})} disabledKeys={['FoR', 'MaR', 'Emp']}>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR"><Edit /><Text>Founding of Rome</Text></Tab>
<Tab id="FoR">Founding of Rome</Tab>
<Tab id="MaR">Monarchy and Republic</Tab>
<Tab id="Emp">Empire</Tab>
</TabList>
Expand All @@ -78,9 +78,9 @@ export const Disabled = (args: any) => (
</div>
);

export const Icons = (args: any) => (
<div className={style({width: 700, height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<Tabs {...args} styles={style({width: 'full'})} iconOnly>
const IconsRender = (props) => (
<div className={style({width: 700, maxWidth: '[calc(100vw - 60px)]', height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<Tabs {...props} styles={style({width: 'full'})}>
<TabList aria-label="History of Ancient Rome">
<Tab id="FoR" aria-label="Edit"><Edit /><Text>Founding of Rome</Text></Tab>
<Tab id="MaR" aria-label="Notifications"><Bell /><Text>Monarchy and Republic</Text></Tab>
Expand All @@ -99,6 +99,10 @@ export const Icons = (args: any) => (
</div>
);

export const Icons = {
render: (args) => <IconsRender {...args} />
Comment thread
snowystinger marked this conversation as resolved.
};

interface Item {
id: number,
title: string,
Expand All @@ -111,7 +115,7 @@ let items: Item[] = [
];

export const Dynamic = (args: any) => (
<div className={style({width: 700, height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<div className={style({width: 700, maxWidth: '[calc(100vw - 60px)]', height: 256, resize: 'horizontal', overflow: 'hidden', padding: 8})}>
<Tabs {...args} styles={style({width: 'full'})} disabledKeys={new Set([2])}>
<TabList aria-label="History of Ancient Rome" items={items}>
{item => <Tab>{item.title}</Tab>}
Expand Down