Skip to content

home screen: Use material tabs for bottom navigation. #4176

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

Open
agrawal-d wants to merge 1 commit into
base: main
Choose a base branch
from
Open

home screen: Use material tabs for bottom navigation. #4176

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions src/main/MainTabs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* @flow strict-local */
import React from 'react';
import { Platform } from 'react-native';
import { createBottomTabNavigator } from 'react-navigation-tabs';
import { Platform, Dimensions } from 'react-native';
import { createMaterialTopTabNavigator } from 'react-navigation-tabs';

import type { TabNavigationOptionsPropsType } from '../types';
import tabsOptions from '../styles/tabs';
Expand All @@ -14,7 +14,7 @@ import { OwnAvatar } from '../common';
import IconUnreadConversations from '../nav/IconUnreadConversations';
import ProfileCard from '../account-info/ProfileCard';

export default createBottomTabNavigator(
export default createMaterialTopTabNavigator(
Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jul 1, 2020

Choose a reason for hiding this comment

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

Hmm, interesting. I wonder why this still puts the tabs on the bottom, when it looks like we're asking for it to make a "top" tab navigator here.

To get the buttons to show feedback, I wonder about still using createBottomTabNavigator, but with something special for the tabBarButton setting: https://reactnavigation.org/docs/bottom-tab-navigator/#tabbarbutton

Copy link
Copy Markdown
Member Author

@agrawal-d agrawal-d Jul 2, 2020

Choose a reason for hiding this comment

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

It's at the bottom because I passed this option in line 66:
tabBarPosition: 'bottom'

I tried Touchable from our 'commons', but it did not give any feedback at all - because it changes opacity on tap, and that does not change the appearance in this case, and also causes a weird layout bug for our 'PMs' icon - because it comes with an unread count indicator, and it becomes misplaced.

I feel material tabs are a good option, because they provide good feedback without an additional setup, along with a nice indicator for the active tab.

Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I feel material tabs are a good option

I agree. I think the problem I have is that createMaterialTopTabNavigator is...just that, a thing to get material tabs at the top. From the doc:

A material-design themed tab bar on the top of the screen [...]

Aha, though! After taking a look at the documentation, it looks like there may be a compromise between createBottomTabNavigator and createMaterialTopTabNavigator:

createMaterialBottomTabNavigator

Have you tried that? 🙂

Copy link
Copy Markdown
Member Author

@agrawal-d agrawal-d Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yep, I already tried that. But we don't have access to createMaterialBottomTabNavigator - the package we use - 'react-navigation-tabs' only provides two exports:

The package exports two different navigators:

createBottomTabNavigator: iOS like bottom tabs.
createMaterialTopTabNavigator: Material design themed top tabs with swipe gesture, from react-native-tab-view.

So, I didn't want to add an extra dependency for material bottom tabs when I was able to use material top tabs. Do you want me to add the dependency createMaterialBottomTabNavigator (yarn add @react-navigation/bottom-tabs) or is this fine? I guess if I do add the dependency, I'll also have to create a flow libdef.

I think we should not add an extra dependency when we are able to configure createMaterialTopTabNavigator to do the job, but I'll let you make the final decision.

Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Mm, earlier, I didn't see that createMaterialBottomTabNavigator means adding another dependency (actually two; the doc says to install react-navigation-material-bottom-tabs and react-native-paper). This would still be true after #4114, as createMaterialBottomTabNavigator isn't exposed by the plain react-navigation library. Making a libdef can be a pain, but it can also be pretty straightforward; we have a growing doc about it, which it may be productive to add to.

I'm still a bit torn; @gnprice, do you have thoughts on this? Someday, without declaring it as a breaking change, createMaterialTopTabNavigator could reasonably drop the ability to be hacked into showing bottom tabs instead of top tabs.

I've just tested out createMaterialBottomTabNavigator; it worked with these versions:

  • "react-navigation-material-bottom-tabs": "^0.4.0" (any later, and there are issues with its dependencies or peerDependencies)
  • "react-native-paper": "^2.0.1" (react-navigation-material-bottom-tabs has this in its peerDependencies)

Just replacing createBottomTabNavigator with createMaterialBottomTabNavigator, and not adding any additional config, this is what it looks like on Android and iOS. It's quite different from what we have now, but there's lots of config available:

On Android, there is a "ripple" effect, like the animation at this doc except the colors don't change (looks like you can optionally give a different color to each tab). It looks like there's no feedback, by default, on iOS. (The latest version explicitly documents a way to switch out the TouchableWithoutFeedback component) .

I do slightly prefer the way it shows which tab is selected; in particular, I like seeing the smooth appearance and disappearance of the tab's name in addition to the icon. We likely wouldn't choose this indigo color, but I like how the icons are translucent when not focused.

Copy link
Copy Markdown
Collaborator

@chrisbobbe chrisbobbe Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

We likely wouldn't choose this indigo color

Heh, though it does more closely match the new logo we now have! #4200

Copy link
Copy Markdown
Member Author

@agrawal-d agrawal-d Jul 23, 2020

Choose a reason for hiding this comment

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

Luckily, our app uses global constants for the theme colours everywhere, so it will be very easy to change the theme colours in other places too.

{
home: {
screen: HomeTab,
Expand Down Expand Up @@ -62,6 +62,8 @@ export default createBottomTabNavigator(
},
{
backBehavior: 'none',
initialLayout: { width: Dimensions.get('window').width },
Copy link
Copy Markdown
Member Author

@agrawal-d agrawal-d Jul 1, 2020

Choose a reason for hiding this comment

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

This is required to prevent a layout bug that happens in the first few moments of initial render.

tabBarPosition: 'bottom',
...tabsOptions({
showLabel: !!Platform.isPad,
showIcon: true,
Expand Down