From 494e01c0a12baad6fc42874a4dfb01f835c15eb7 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 5 Dec 2024 10:00:20 +1100 Subject: [PATCH] fix: Tabs from testing (#7463) * fix: Tabs from testing * fix horizontal spacing for icon only * more spacing fixes * make mobile testing easier * rename prop isIconOnly --- .../s2/chromatic/Tabs.stories.tsx | 10 ++--- packages/@react-spectrum/s2/src/Tabs.tsx | 37 ++++++++++++------- .../@react-spectrum/s2/src/TabsPicker.tsx | 29 +++++++++++---- .../s2/stories/Tabs.stories.tsx | 20 ++++++---- 4 files changed, 63 insertions(+), 33 deletions(-) diff --git a/packages/@react-spectrum/s2/chromatic/Tabs.stories.tsx b/packages/@react-spectrum/s2/chromatic/Tabs.stories.tsx index 3525c5c3785..80a3c6f9399 100644 --- a/packages/@react-spectrum/s2/chromatic/Tabs.stories.tsx +++ b/packages/@react-spectrum/s2/chromatic/Tabs.stories.tsx @@ -31,7 +31,7 @@ export default meta; export const Example = (args: any) => ( - Founding of Rome + Founding of Rome Monarchy and Republic Empire @@ -57,9 +57,9 @@ export const Example = (args: any) => ( export const Disabled = (args: any) => ( - Founding of Rome - Monarchy and Republic - Empire + Edit + Notifications + Likes Arma virumque cano, Troiae qui primus ab oris. @@ -74,7 +74,7 @@ export const Disabled = (args: any) => ( ); export const Icons = (args: any) => ( - + Edit Notifications diff --git a/packages/@react-spectrum/s2/src/Tabs.tsx b/packages/@react-spectrum/s2/src/Tabs.tsx index 2b9dcd250b2..04de2f8bd7f 100644 --- a/packages/@react-spectrum/s2/src/Tabs.tsx +++ b/packages/@react-spectrum/s2/src/Tabs.tsx @@ -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 @@ -55,7 +55,7 @@ export interface TabsProps extends Omit, StyleProps { @@ -96,7 +96,7 @@ export const Tabs = forwardRef(function Tabs(props: TabsProps, ref: DOMRef pickerRef.current?.focus(), pickerRef }] @@ -170,7 +170,7 @@ const tablist = style({ }); export function TabList(props: TabListProps) { - 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(undefined); @@ -208,7 +208,7 @@ export function TabList(props: TabListProps) { tablist({...renderProps, isIconOnly: iconOnly, density})} /> + className={renderProps => tablist({...renderProps, isIconOnly, density})} /> {orientation === 'horizontal' && } @@ -255,7 +255,7 @@ const selectedIndicator = style({ transitionTimingFunction: 'in-out' }); -function TabLine(props: TabLineProps) { +function TabLine(props: TabLineProps & {isIconOnly?: boolean}) { let { disabledKeys, isDisabled: isTabsDisabled, @@ -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(selectedTab); + // assign ref before the useResizeObserver useEffect runs + useLayoutEffect(() => { + ref.current = selectedTab; + }); + useResizeObserver({ref, onResize}); return (
@@ -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({ @@ -346,7 +356,7 @@ const icon = style({ }); export function Tab(props: TabProps) { - let {density, iconOnly} = useContext(InternalTabsContext) ?? {}; + let {density, isIconOnly} = useContext(InternalTabsContext) ?? {}; return ( (props.UNSAFE_className || '') + tab({...renderProps, density}, props.styles)}> + className={renderProps => (props.UNSAFE_className || '') + tab({...renderProps, density, isIconOnly}, props.styles)}> {({ // @ts-ignore isMenu @@ -372,7 +382,7 @@ export function Tab(props: TabProps) { display: { isIconOnly: 'none' } - })({isIconOnly: iconOnly}) + })({isIconOnly}) }], [IconContext, { render: centerBaseline({slot: 'icon', styles: style({order: 0})}), @@ -469,7 +479,7 @@ let HiddenTabs = function (props: { let TabsMenu = (props: {items: Array>, 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()); @@ -491,6 +501,7 @@ let TabsMenu = (props: {items: Array>, onSelectionChange: TabsProps['o ref={pickerRef ? pickerRef : undefined} isDisabled={isDisabled || allKeysDisabled} density={density!} + isIconOnly={isIconOnly} items={items} disabledKeys={disabledKeys} selectedKey={selectedKey} diff --git a/packages/@react-spectrum/s2/src/TabsPicker.tsx b/packages/@react-spectrum/s2/src/TabsPicker.tsx index 313c626876c..c66c8c264e0 100644 --- a/packages/@react-spectrum/s2/src/TabsPicker.tsx +++ b/packages/@react-spectrum/s2/src/TabsPicker.tsx @@ -29,7 +29,6 @@ import { checkmark, description, icon, - iconCenterWrapper, label, menuitem, sectionHeader, @@ -37,7 +36,7 @@ import { } 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 @@ -86,7 +85,11 @@ export interface PickerProps 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>, FocusableRefValue>>(null); @@ -155,6 +158,14 @@ const iconStyles = style({ } }); +const iconCenterWrapper = style({ + display: 'flex', + gridArea: 'icon', + paddingStart: { + isIconOnly: size(6) + } +}); + let InsideSelectValueContext = createContext(false); function Picker(props: PickerProps, ref: FocusableRef) { @@ -170,6 +181,7 @@ function Picker(props: PickerProps, ref: FocusableRef(props: PickerProps, ref: FocusableRef(props: PickerProps, ref: FocusableRef = { export default meta; export const Example = (args: any) => ( -
+
- Founding of Rome + Founding of Rome Monarchy and Republic Empire @@ -58,10 +58,10 @@ export const Example = (args: any) => ( ); export const Disabled = (args: any) => ( -
+
- Founding of Rome + Founding of Rome Monarchy and Republic Empire @@ -78,9 +78,9 @@ export const Disabled = (args: any) => (
); -export const Icons = (args: any) => ( -
- +const IconsRender = (props) => ( +
+ Founding of Rome Monarchy and Republic @@ -99,6 +99,10 @@ export const Icons = (args: any) => (
); +export const Icons = { + render: (args) => +}; + interface Item { id: number, title: string, @@ -111,7 +115,7 @@ let items: Item[] = [ ]; export const Dynamic = (args: any) => ( -
+
{item => {item.title}}