-
-
Notifications
You must be signed in to change notification settings - Fork 612
feat(Tabs): allow full tabs background color styling #3492
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
feat(Tabs): allow full tabs background color styling #3492
Conversation
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've watched the videos carefully and here are my initial thoughts:
Please take a look at the "bg_only_react" video. Notice that after you press on "tab 2" (first change to "tab 2"), the view does not change immediately (or maybe it does, but it is fully transparent, as there is no content? - this is likely what happens). This leads to situation where we get this seemingly nice transition w/o effect where content appears on previously empty background after a second. I'm pretty sure, that in case the render is heavy (takes like 500 ms) we'll observe that tab changes nicely, but you still can see content from previous screen for those couple hundreds of ms. So this approach does not solve the initial problem, it mitigates it by accidentally displaying transparent content until React mounts rendered views.
I'll give code remarks & suggestions in separate review
|
Also from these videos & PR description I do not quite understand impact of setting background colour on |
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My current understanding is that with leaving the main container view transparent and setting the background colour for the "TabScreenView" only after first content render we could mitigate the problems pretty nicely?
But would glass effects on tab bar behave nicely in such configuration?
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code itself looks good. I don't have any bigger remarks. But let's discuss the approach first ☝🏻
You're right here. What happens is:
Here is the video with heavy tab2 render: bg_only_react_heavy.mov
This is relevant when we try "bg-react-only" approach with transparent bg_react_container_heavy.movAlso, even if the tab content has been rendered, the container background is visible during the transition. Here is the video (container background is blue as well): bg_react_container_switch.movI haven't tested yet how tab bar appearance adaptation works with each of those approaches but previously having solid background for
I'm not exactly sure what you mean here. "bg-only-tab"/"bg-tab-and-react" + preload should behave well. |
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good. We must only make sure to coordinate the changes with #3535
Let's wait for that PR first & then adjust the naming here.
| export type TabBarControllerMode = 'automatic' | 'tabBar' | 'tabSidebar'; | ||
|
|
||
| export interface BottomTabsProps extends ViewProps { | ||
| export type BottomTabsNativeContainerStyleProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's refactor this PR after #3535 is merged.
|
I've merged #3535. We can rebase / merge main to this PR and proceed here. |
4961f4c to
ccdef34
Compare
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
Adds customization of
backgroundColorfor native tabs container (UITabBarController.viewon iOS,TabsHoston Android) and tab screen (RNSBottomTabsScreenComponentViewon iOS,TabScreenon Android).3492_Android.mp4
3492_iOS.mov
Closes https://github.com/software-mansion/react-native-screens-labs/issues/721.
Supersedes #3478.
Comparison of different approaches to setting background color on iOS
This PR allows for (at least) 3 kinds of setting background color in tabs. Due to asynchronous nature, different approaches result in different effect.
Tab screen + React View has background color
This is similar to current approach where tab screen uses
systemBackgroundColorand user defines background color via View inside components passed to tab screen. This approach was used to mitigate problems with liquid glass tab bar appearance adaptation (more details: #3279).bg_tab_and_react.mov
First, background color of tab screen is visible before tab's content is rendered by React and correct color appears.
Only React View has background color
This is similar to approach suggested in #3478 (transparent screen). While this seems to work better on first tab renders, switching between similar tabs after they are rendered causes a flash.
bg_only_react.mov
This is reproducible in bare UIKit app when child VC's view has clear background color and its subview has different background color.
Only Tab screen has background color
This approach is possible thanks to this PR and it is closes to native platform but tab's content still appears after a delay because it needs to be rendered by React - this causes a flash on first render.
bg_only_tab.mov
Changes
nativeContainerStyleprop toBottomTabscomponent withbackgroundColorfieldstyleprop toBottomTabsScreencomponent withbackgroundColorfieldTest3492Test code and steps to reproduce
Run
Test3492.Checklist