Skip to content

TabController - remove from incubator and remove support for children in TabBar #1182

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

Merged
merged 1 commit into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion demo/src/screens/componentScreens/TabBarScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default class TabBarScreen extends Component {

<TabBar style={styles.tabbar} selectedIndex={0} ref={r => (this.tabbar = r)} enableShadow>
<TabBar.Item label="Scroll"/>
<TabBar.Item label="View" badge={{size: 6}}/>
<TabBar.Item label="View" badgeProps={{size: 6}}/>
<TabBar.Item label="tab"/>
<TabBar.Item label="bar"/>
<TabBar.Item label="Container"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class TabControllerScreen extends Component<{}, State> {

const addItem: TabControllerItemProps = {icon: Assets.icons.demo.add, key: 'add', ignore: true, width: 60, onPress: this.onAddItem};

return [...items, addItem];
return fewItems ? items : [...items, addItem];
};

componentDidMount() {
Expand Down Expand Up @@ -154,9 +154,7 @@ class TabControllerScreen extends Component<{}, State> {
enableShadow
activeBackgroundColor={Colors.blue60}
centerSelected={centerSelected}
>
{/* {this.renderTabItems()} */}
</TabController.TabBar>
/>
{this.renderTabPages()}
</TabController>
<View absB left margin-20 marginB-100 style={{zIndex: 1}}>
Expand Down
4 changes: 2 additions & 2 deletions generatedTypes/components/badge/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export declare type BadgeProps = ViewProps & TouchableOpacityProps & {
/**
* the badge size (default, small)
*/
size: number;
size?: number;
/**
* Press handler
*/
Expand Down Expand Up @@ -344,7 +344,7 @@ declare const _default: React.ComponentClass<ViewProps & TouchableOpacityProps &
/**
* the badge size (default, small)
*/
size: number;
size?: number | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to specify undefined type for an optional prop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't, this is the automatically generated .d.ts file (when running npm run build:dev).

/**
* Press handler
*/
Expand Down
1 change: 0 additions & 1 deletion generatedTypes/incubator/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export { default as TabController } from './TabController';
export { default as TextField } from './TextField';
export { default as TouchableOpacity, TouchableOpacityProps } from './TouchableOpacity';
export { default as WheelPicker } from './WheelPicker';
2 changes: 1 addition & 1 deletion src/components/badge/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export type BadgeProps = ViewProps &
/**
* the badge size (default, small)
*/
size: number;
size?: number;
/**
* Press handler
*/
Expand Down
92 changes: 16 additions & 76 deletions src/components/tabController/TabBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {asBaseComponent, forwardRef, BaseComponentInjectedProps, ForwardRefInjec
import View from '../../components/view';
import {Colors, Spacings, Typography} from '../../style';
import {Constants} from '../../helpers';
import {LogService} from '../../services';
import FadedScrollView from './FadedScrollView';

import {useScrollToItem} from '../../hooks';
Expand Down Expand Up @@ -114,11 +113,7 @@ export interface TabControllerBarProps {
testID?: string;
}

type ChildProps = React.ReactElement<TabControllerItemProps>;

interface Props extends TabControllerBarProps, BaseComponentInjectedProps, ForwardRefInjectedProps {
children?: ChildProps[] | ChildProps;
}
interface Props extends TabControllerBarProps, BaseComponentInjectedProps, ForwardRefInjectedProps {}

/**
* @description: TabController's TabBar component
Expand All @@ -144,47 +139,27 @@ const TabBar = (props: Props) => {
containerWidth: propsContainerWidth,
centerSelected,
containerStyle,
testID,
children: propsChildren
testID
} = props;

const context = useContext(TabBarContext);
// @ts-ignore // TODO: typescript
const {itemStates, items: contextItems, currentPage, targetPage, registerTabItems, selectedIndex} = context;

const children = useRef<Props['children']>(_.filter(propsChildren, (child: ChildProps) => !!child));

const _registerTabItems = () => {
const ignoredItems: number[] = [];
let itemsCount;

if (propsItems) {
itemsCount = _.size(propsItems);
_.forEach(propsItems, (item, index) => {
if (item.ignore) {
ignoredItems.push(index);
}
});
// TODO: deprecate with props.children
} else {
itemsCount = React.Children.count(children.current);
// @ts-ignore TODO: typescript - not sure if this can be solved
React.Children.toArray(children.current).forEach((child: ChildProps, index: number) => {
if (child.props.ignore) {
ignoredItems.push(index);
}
});
}
const itemsCount = _.size(propsItems);
_.forEach(propsItems, (item, index) => {
if (item.ignore) {
ignoredItems.push(index);
}
});

registerTabItems(itemsCount, ignoredItems);
};

useEffect(() => {
if (propsChildren) {
LogService.warn('uilib: Please pass the "items" prop to TabController.TabBar instead of children');
}

if ((propsItems || children.current) && !contextItems) {
if (propsItems && !contextItems) {
_registerTabItems();
}
}, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no dependencies here, like 'items'? Won't it cause unnecessary updates?

Copy link
Collaborator Author

@M-i-k-e-l M-i-k-e-l Feb 10, 2021

Choose a reason for hiding this comment

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

On the contrary, no dependencies means no re-renders (this is similar to componentDidMount).

Expand All @@ -196,7 +171,7 @@ const TabBar = (props: Props) => {
return contextItems || propsItems;
}, [contextItems, propsItems]);

const itemsCount = useRef<number>(items ? _.size(items) : React.Children.count(children.current));
const itemsCount = useRef<number>(_.size(items));

const {scrollViewRef: tabBar, onItemLayout, itemsWidths, focusIndex} = useScrollToItem({
itemsCount: itemsCount.current,
Expand All @@ -216,7 +191,11 @@ const TabBar = (props: Props) => {
return offsets;
}, [itemsWidths]);

const _renderTabBarItems = useMemo((): ReactNode => {
const renderTabBarItems = useMemo((): ReactNode => {
if (_.isEmpty(itemStates)) {
return null;
}

return _.map(items, (item, index) => {
return (
<TabBarItem
Expand All @@ -239,6 +218,7 @@ const TabBar = (props: Props) => {
);
});
}, [
items,
labelColor,
selectedLabelColor,
labelStyle,
Expand All @@ -252,46 +232,6 @@ const TabBar = (props: Props) => {
onItemLayout
]);

// TODO: Remove once props.children is deprecated
const _renderTabBarItemsFromChildren = useMemo((): ReactNode | null => {
return !children.current
? null
: React.Children.map(children.current, (child: Partial<ChildProps>, index: number) => {
// @ts-ignore TODO: typescript - not sure if this can be easily solved
return React.cloneElement(child, {
labelColor,
selectedLabelColor,
labelStyle,
selectedLabelStyle,
uppercase,
iconColor,
selectedIconColor,
activeBackgroundColor,
...child.props,
...context,
index,
state: itemStates[index],
onLayout: centerSelected ? onItemLayout : undefined
});
});
}, [
labelColor,
selectedLabelColor,
labelStyle,
selectedLabelStyle,
uppercase,
iconColor,
selectedIconColor,
activeBackgroundColor,
itemStates,
centerSelected,
onItemLayout
]);

const renderTabBarItems = useMemo(() => {
return _.isEmpty(itemStates) ? null : items ? _renderTabBarItems : _renderTabBarItemsFromChildren;
}, [itemStates, items, _renderTabBarItems, _renderTabBarItemsFromChildren]);

const _indicatorWidth = new Value(0); // TODO: typescript?
const _indicatorOffset = new Value(0); // TODO: typescript?

Expand Down
63 changes: 0 additions & 63 deletions src/incubator/TabController/PageCarousel.js

This file was deleted.

41 changes: 0 additions & 41 deletions src/incubator/TabController/ReanimatedObject.js

This file was deleted.

Loading