Skip to content

Commit

Permalink
Re-structure TabList Animated Indicator code to work better with RTL …
Browse files Browse the repository at this point in the history
…layouts (#3236)

* Fix RTL styling on win32

* Change files

* Fix RTL rendering on mac

* Fix snapshot tests failing

* Re-add root slot to fix indicator positioning

* Rewrite animated indicator to work better with RTL

This commit simplifies the logic of the animated indicator. Rather than
positioning the indicator using a calculated `start` value, we position
it using absolute layout values within the tablist. Now, the
`useTabAnimation` hook does all the math to figure out the top and left
offsets of the indicator, and the styling hook for the animatedIndicator
simply uses them to position the `top` and `left` layout props.

The container slot of the animated indicator is also removed.

* Clean-up: refactoring + adding comments

* Fix snapshot tests

* Fix tablist test page title again

* Fix wrong token setting indicator border radius
  • Loading branch information
lawrencewin authored Dec 13, 2023
1 parent d05effc commit 942da04
Show file tree
Hide file tree
Showing 16 changed files with 2,173 additions and 2,090 deletions.
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>
{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

0 comments on commit 942da04

Please sign in to comment.