Skip to content
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

fix: Fix android statusbar translucency #374

Merged

Conversation

DragonSpirit
Copy link
Contributor

@DragonSpirit DragonSpirit commented Dec 21, 2019

Sup!
This PR should fix transparency issue from #321. It would be great if @vikrantnegi or anybody else can check this.

@amaury1093 amaury1093 changed the title Fix android statusbar translucency fix: Fix android statusbar translucency Dec 22, 2019
Copy link
Member

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

looks good to me, thanks! I'll let @vikrantnegi review if he sees this, or else I'll merge this in 2d.

@vikrantnegi
Copy link
Contributor

@DragonSpirit Hey, I just reviewd the PR on my Android and unfortunately it is not seems to working as expected.

However, when I added barStyle to the StatusBar it worked.

<StatusBar translucent={false} barStyle="dark-content" />

cc @amaurymartiny

@DragonSpirit
Copy link
Contributor Author

@vikrantnegi I am not sure it's correct way for light-based theme devices. Main point is disable transparency from statusbar. Can you attach screenshot for check difference?

@vikrantnegi
Copy link
Contributor

Good point. But we can always update the bar style as per the app theme like this:

<StatusBar
  barStyle={
    this.state.theme === 'light'
      ? 'dark-content'
      : 'light-content'
  }
  backgroundColor={
    this.state.theme === 'light' ? '#fff' : '#000'
  }
/>

I implemented this in one of my expo apps.

Here is the screenshots before and after adding the barStyle.
Before:
Before-barStyle

After:
After-barStyle

@DragonSpirit
Copy link
Contributor Author

@vikrantnegi, thank you for research. But I think theme feature for statusbar should be implemented in #320. Use force dark-content can break iOS dark theme.
@amaurymartiny what do you think about it?

@vikrantnegi
Copy link
Contributor

@DragonSpirit Using dark-content won't break iOS dark theme as the app doesn't support dark theme, so iOS won't apply dark theme to the app.

Copy link
Member

@amaury1093 amaury1093 left a comment

Choose a reason for hiding this comment

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

Use force dark-content can break iOS dark theme.

I believe @vikrantnegi is actually correct. By default the light theme is used everywhere (see here), so we can just put <StatusBar barStyle="dark-content"> and it should be enough.

In the future, after #320, we can enable automatic dark/light detection and add a piece of code similar to #374 (comment)

App/Screens/Screens.tsx Outdated Show resolved Hide resolved
@amaury1093
Copy link
Member

Thanks a lot @DragonSpirit and @vikrantnegi!

@amaury1093 amaury1093 merged commit 79f3345 into shootismoke:master Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants