fix: Dynamic status bar #641
Conversation
There was a problem hiding this comment.
@jouderianjr 👍 LGTM! And I also left a personal suggestion for you.
App.js
Outdated
| StatusBar.setTranslucent(false); | ||
| StatusBar.setBackgroundColor(colors.grey); | ||
| StatusBar.setBarStyle('dark-content'); | ||
| } |
There was a problem hiding this comment.
@jouderianjr Can we config these informations in somewhere? So that we will not forget to update them once new routers were added.
|
Awesome guys, until the end of the week I will fix it 💃 |
|
Finally! 👯♂️ |
App.js
Outdated
| if (lightContentScreens.includes(routeName)) { | ||
| StatusBar.setTranslucent(true); | ||
| StatusBar.setBackgroundColor( | ||
| routeName === 'Login' ? colors.transparent : colors.black |
There was a problem hiding this comment.
Can you replace colors.black by #262626. This would Android look like the iPhone:
Before:
After:
There was a problem hiding this comment.
Are you suggesting we make a new color for this? Or replace all blacks by #262626?
There was a problem hiding this comment.
We should make a new color in config for this.
The existant dark gray on profile pages is already #262626 but created with a rgba() thingy (that seems unneeded)
App.js
Outdated
| StatusBar.setBarStyle('light-content'); | ||
| } else { | ||
| StatusBar.setTranslucent(false); | ||
| StatusBar.setBackgroundColor(colors.grey); |
There was a problem hiding this comment.
colors.white look nicer to me (give the same transparent look on Android)
There was a problem hiding this comment.
I completely forgot about material design 😅
Let's keep it like you did it then
|
@andrewda @chinesedfan Looks better? 💃 If not, I will glad to hear some suggestions, I don't know if is the best implementation for this. 😄 |
App.js
Outdated
|
|
||
| const statusBarConfig = lightScreens.includes(routeName) | ||
| ? lightStatusBar | ||
| : darkStatusBar; |
There was a problem hiding this comment.
@jouderianjr How about moving these lines into src/config/status-bar.js and only exporting 1 function called getStatusBarConfig(routerName)? Suppose the 3rd kind of status bar was introduced, only the config file should be updated.
|
@chinesedfan Done! Good suggestion mate |
App.js
Outdated
| ); | ||
|
|
||
| StatusBar.setTranslucent(translucent); | ||
| StatusBar.setBackgroundColor(backgroundColor(routeName)); |
There was a problem hiding this comment.
@jouderianjr Please return backgroundColor value, instead of a function. Except here everything is so good!
There was a problem hiding this comment.
backgroundColor in lightStatusBar depends of if you are in Login or not. But I have an Idea, I can transform lightStatusBar and darkStatusBar in a function that receives the routeName and return the object.
Example:
const lightStatusBar = routeName => {
translucent: true,
backgroundColor: routeName === 'Login' ? colors.transparent : colors.black,
barStyle: 'light-content',
};
What do you think about?
There was a problem hiding this comment.
And you may rename lightStatusBar as getLightStatusBar.
jouderianjr
force-pushed
the
dynamic_status_bar
branch
from
d68319c
to
a4e2609
Compare
|
done @chinesedfan 💃 |
|
I think it good to go now @chinesedfan @machour @andrewda 💃 |
|
Just tested and validated with latest modifications. Merging this baby in! |
Screenshots
Description
The main idea here is using the
onNavigationStateChangeevent for discovering what screen the user is navigating and change the statusBarStyle accordingly with the screen.