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.

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

Re-structure TabList Animated Indicator code to work better with RTL layouts #3236

Merged
merged 11 commits into from
Dec 13, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -227,5 +227,5 @@ export const TabListTest: React.FunctionComponent = () => {

const description = 'With Tabs, users can navigate to another view.';

return <Test name="TabsV1 Test" description={description} sections={sections} status={status} e2eSections={e2eSections} />;
return <Test name="TabList Test" description={description} sections={sections} status={status} e2eSections={e2eSections} />;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Change TabList test page title to be correct",
"packageName": "@fluentui-react-native/tablist",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix RTL styling on win32",
"packageName": "@fluentui-react-native/tester",
"email": "[email protected]",
"dependentChangeType": "patch"
}
3 changes: 2 additions & 1 deletion packages/experimental/TabList/src/Tab/Tab.styling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ export const useTabSlotProps = (props: TabProps, tokens: TabTokens, theme: Theme
display: 'flex',
alignItems: 'center',
flexDirection: 'row',
flex: vertical ? 0 : 1,
alignSelf: 'flex-start',
justifyContent: 'center',
marginHorizontal: tokens.stackMarginHorizontal,
marginVertical: tokens.stackMarginVertical,
},
}),
[tokens.stackMarginHorizontal, tokens.stackMarginVertical],
[vertical, tokens.stackMarginHorizontal, tokens.stackMarginVertical],
);

const indicatorContainer = React.useMemo<IViewProps>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ exports[`Tab component tests Customized Tab 1`] = `
"alignItems": "center",
"alignSelf": "flex-start",
"display": "flex",
"flex": 1,
"flexDirection": "row",
"justifyContent": "center",
"marginHorizontal": 6,
Expand Down Expand Up @@ -217,6 +218,7 @@ exports[`Tab component tests Tab default props 1`] = `
"alignItems": "center",
"alignSelf": "flex-start",
"display": "flex",
"flex": 1,
"flexDirection": "row",
"justifyContent": "center",
"marginHorizontal": 6,
Expand Down Expand Up @@ -353,6 +355,7 @@ exports[`Tab component tests Tab disabled 1`] = `
"alignItems": "center",
"alignSelf": "flex-start",
"display": "flex",
"flex": 1,
"flexDirection": "row",
"justifyContent": "center",
"marginHorizontal": 6,
Expand Down Expand Up @@ -489,6 +492,7 @@ exports[`Tab component tests Tab render icon + text 1`] = `
"alignItems": "center",
"alignSelf": "flex-start",
"display": "flex",
"flex": 1,
"flexDirection": "row",
"justifyContent": "center",
"marginHorizontal": 6,
Expand Down Expand Up @@ -637,6 +641,7 @@ exports[`Tab component tests Tab render icon only 1`] = `
"alignItems": "center",
"alignSelf": "flex-start",
"display": "flex",
"flex": 1,
"flexDirection": "row",
"justifyContent": "center",
"marginHorizontal": 6,
Expand Down
61 changes: 36 additions & 25 deletions packages/experimental/TabList/src/Tab/useTabAnimation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Platform } from 'react-native';
import { I18nManager, Platform } from 'react-native';

import type { LayoutEvent, PressablePropsExtended } from '@fluentui-react-native/interactive-hooks';

Expand Down Expand Up @@ -32,52 +32,63 @@ export function useTabAnimation(
// If we're the selected tab, we style the TabListAnimatedIndicator with the correct token value set by the user
React.useEffect(() => {
if (tabKey === selectedKey && updateAnimatedIndicatorStyles) {
updateAnimatedIndicatorStyles({ indicator: { backgroundColor: tokens.indicatorColor } });
updateAnimatedIndicatorStyles({ backgroundColor: tokens.indicatorColor, borderRadius: tokens.indicatorRadius });
}
// Disabling warning because effect does not need to fire on `updateAnimatedIndicatorStyles` being changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [tabKey, selectedKey, tokens.indicatorColor]);
}, [tabKey, selectedKey, tokens.indicatorColor, tokens.indicatorRadius]);

/**
* This checks to see if we have relevant info to calculate the layout position and dimensions of the indicator. If this check fails, we don't
* want to trigger a re-render by needlessly updating the TabList state.
*
* We also check if the info is good. Info can be bad in some weird cases:
* We also check if the info is good. Info can be bad in some weird cases on win32:
* - Check if width > 0 because there is an on-going issue caused by ScrollViews initially laying out its childrens' width to 0 and height to be a bigger than expected value.
* - ScrollView also negatively affects the initial height values. For vertical TabLists, the initial height value will lay out incorrectly. Sometimes, the styling of the parent
* component combined with the ScrollView issues causes the initial height layout value to be completely unreasonable. Exactly which style that causes this issue isn't known;
* more investigation has to be done.
*
* Once we finish these checks, for each tab, we calculate the layout information of its indicator consisting of (1) its dimensions and (2) its position (x,y) relative to the tablist.
* Afterwards, we save these to feed into the Animated Indicator's layout styles.
*/
const onTabLayout = React.useCallback(
(e: LayoutEvent) => {
if (
e.nativeEvent.layout &&
// Following checks are for win32 only, will be removed after addressing scrollview layout bug
(Platform.OS !== ('win32' as any) ||
(layout?.tablist &&
layout?.tablist.width > 0 &&
e.nativeEvent.layout.height <= layout.tablist.height &&
e.nativeEvent.layout.height < RENDERING_HEIGHT_LIMIT))
(e.nativeEvent.layout &&
// Following checks are for win32 only, will be removed after addressing scrollview layout bug
Platform.OS !== ('win32' as any)) ||
(layout?.tablist &&
layout.tablist.width > 0 &&
e.nativeEvent.layout.height <= layout.tablist.height &&
e.nativeEvent.layout.height < RENDERING_HEIGHT_LIMIT)
) {
let width: number, height: number;
const { width: tabWidth, height: tabHeight, x: tabX, y: tabY } = e.nativeEvent.layout;
let indicatorWidth: number, indicatorHeight: number, indicatorX: number, indicatorY: number;
// Total Indicator inset consists of the horizontal/vertical margin of the indicator, the space taken up by the tab's focus border, and the
// existing padding between the focus border and the tab itself. Multiply by 2 to account for the start + end margin/border/padding.
// existing padding between the focus border and the tab itself.
const focusBorderPadding = 1;
const totalIndicatorInset = 2 * (tokens.indicatorMargin + tokens.borderWidth + focusBorderPadding);
// we can calculate the dimensions of the indicator using token values we have access to.
const totalIndicatorInset = tokens.indicatorMargin + tokens.borderWidth + focusBorderPadding;
if (vertical) {
width = tokens.indicatorThickness;
height = e.nativeEvent.layout.height - totalIndicatorInset;
indicatorWidth = tokens.indicatorThickness;
indicatorHeight = tabHeight - totalIndicatorInset * 2; // multiply inset by 2 to subtract height from top and bottom
indicatorY = tabY + totalIndicatorInset;
if (I18nManager.isRTL) {
// On RTL, the vertical tab indicator should appear to the right of the text
indicatorX = tabX + tabWidth - (tokens.borderWidth + focusBorderPadding + indicatorWidth);
} else {
indicatorX = tabX + tokens.borderWidth + focusBorderPadding;
}
} else {
width = e.nativeEvent.layout.width - totalIndicatorInset;
height = tokens.indicatorThickness;
indicatorWidth = tabWidth - totalIndicatorInset * 2; // multiply inset by 2 to subtract width from left and right
indicatorHeight = tokens.indicatorThickness;
indicatorX = tabX + totalIndicatorInset;
indicatorY = tabHeight + tabY - indicatorHeight - tokens.borderWidth - focusBorderPadding;
}
addTabLayout(tabKey, {
x: e.nativeEvent.layout.x,
y: e.nativeEvent.layout.y,
width: width,
height: height,
tabBorderWidth: tokens.borderWidth,
startMargin: tokens.indicatorMargin,
x: indicatorX,
y: indicatorY,
width: indicatorWidth,
height: indicatorHeight,
});
}
},
Expand Down
15 changes: 13 additions & 2 deletions packages/experimental/TabList/src/TabList/TabList.styling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,24 @@ export const stylingSettings: UseStylingOptions<TabListProps, TabListSlotProps,
states: ['vertical'],
slotProps: {
stack: buildProps(
(tokens: TabListTokens, theme: Theme) => ({
(tokens: TabListTokens) => ({
style: {
display: 'flex',
flexDirection: tokens.direction,
flex: 0,
},
}),
['direction'],
),
root: buildProps(
(tokens: TabListTokens, theme: Theme) => ({
style: {
display: 'flex',
alignItems: 'flex-start',
...layoutStyles.from(tokens, theme),
},
}),
['direction', ...layoutStyles.keys],
layoutStyles.keys,
),
},
};
35 changes: 19 additions & 16 deletions packages/experimental/TabList/src/TabList/TabList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const TabList = compose<TabListType>({
slots: {
container: FocusZone,
stack: View,
root: View,
},
useRender: (userProps: TabListProps, useSlots: UseSlots<TabListType>) => {
// configure props and state for tabs based on user props
Expand All @@ -43,22 +44,24 @@ export const TabList = compose<TabListType>({
// Passes in the selected key and a hook function to update the newly selected tab and call the client's onTabsClick callback.
value={tablist.state}
>
<Slots.container
disabled={disabled || tablistDisabledState}
defaultTabbableElement={defaultTabbableElement}
focusZoneDirection={vertical ? 'vertical' : 'horizontal'}
isCircularNavigation={isCircularNavigation}
>
<Slots.stack {...mergedProps}>{children}</Slots.stack>
{canShowAnimatedIndicator && (
<TabListAnimatedIndicator
animatedIndicatorStyles={animatedIndicatorStyles}
selectedKey={selectedKey}
tabLayout={layout.tabs}
vertical={vertical}
/>
)}
</Slots.container>
<Slots.root {...mergedProps}>
<Slots.container
disabled={disabled || tablistDisabledState}
defaultTabbableElement={defaultTabbableElement}
focusZoneDirection={vertical ? 'vertical' : 'horizontal'}
isCircularNavigation={isCircularNavigation}
>
<Slots.stack>{children}</Slots.stack>
lawrencewin marked this conversation as resolved.
Show resolved Hide resolved
{canShowAnimatedIndicator && (
<TabListAnimatedIndicator
animatedIndicatorStyles={animatedIndicatorStyles}
selectedKey={selectedKey}
tabLayout={layout.tabs}
vertical={vertical}
/>
)}
</Slots.container>
</Slots.root>
</TabListContext.Provider>
);
};
Expand Down
15 changes: 6 additions & 9 deletions packages/experimental/TabList/src/TabList/TabList.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@ import type { FocusZoneProps } from '@fluentui-react-native/focus-zone';
import type { LayoutTokens } from '@fluentui-react-native/tokens';
import type { LayoutRectangle } from '@office-iss/react-native-win32';

import type {
AnimatedIndicatorStyles,
AnimatedIndicatorStylesUpdate,
TabLayoutInfo,
} from '../TabListAnimatedIndicator/TabListAnimatedIndicator.types';
import type { AnimatedIndicatorStyles } from '../TabListAnimatedIndicator/TabListAnimatedIndicator.types';

export const tabListName = 'TabList';

export type TabListAppearance = 'transparent' | 'subtle';
export type TabListSize = 'small' | 'medium' | 'large';
export interface TabListLayoutInfo {
tablist: LayoutRectangle;
tabs: { [key: string]: TabLayoutInfo };
tabs: { [key: string]: LayoutRectangle };
}

export interface TabListState {
Expand All @@ -30,7 +26,7 @@ export interface TabListState {
/**
* Method to add Tab's layout information for animating the tab indicator
*/
addTabLayout?: (tabKey: string, layout: TabLayoutInfo) => void;
addTabLayout?: (tabKey: string, layout: LayoutRectangle) => void;

/**
* Global state both TabList and Tab use for tracking styling of the animated indicator.
Expand Down Expand Up @@ -104,7 +100,7 @@ export interface TabListState {
/**
* Directly update the animated indicator's styles with styles the user supplies for each slot.
*/
updateAnimatedIndicatorStyles?: (updates: AnimatedIndicatorStylesUpdate) => void;
updateAnimatedIndicatorStyles?: (updates: AnimatedIndicatorStyles) => void;

/**
* Updates internal map that keeps track of each of this tablist's tabs disabled state
Expand Down Expand Up @@ -184,7 +180,8 @@ export interface TabListInfo {
}
export interface TabListSlotProps {
container?: FocusZoneProps;
stack: React.PropsWithRef<IViewProps>;
stack: IViewProps;
root: React.PropsWithRef<IViewProps>;
}

export interface TabListType {
Expand Down
Loading
Loading