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

feat: allow for providing safe area provider props #526

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Conversation

jkdowdle
Copy link
Contributor

@jkdowdle jkdowdle commented Feb 6, 2024

Changes

Screenshots

@coveralls
Copy link

coveralls commented Feb 6, 2024

Pull Request Test Coverage Report for Build 7805043023

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 85.099%

Totals Coverage Status
Change from base Build 7718841149: 0.0%
Covered Lines: 3662
Relevant Lines: 4181

💛 - Coveralls

Copy link
Member

@aecorredor aecorredor left a comment

Choose a reason for hiding this comment

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

Curious, why did we need this in the FL app?

aecorredor
aecorredor previously approved these changes Feb 6, 2024
aecorredor
aecorredor previously approved these changes Feb 6, 2024
sternetj
sternetj previously approved these changes Feb 6, 2024
Copy link
Contributor

@sternetj sternetj 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. Where we not able to just modify the theme we pass via the DeveloperConfigProvider to update the navigator's theme?

@jkdowdle
Copy link
Contributor Author

jkdowdle commented Feb 6, 2024

Looks good. Where we not able to just modify the theme we pass via the DeveloperConfigProvider to update the navigator's theme?

I think I may have made a mistake. I assumed it was wrong for the themed navigation because the Theme types are incompatible between the two. But they have a lot of overlap at least for my purposes. I think I'll back out of that change

@sternetj you're right, we can just change the value in our passed theme

@jkdowdle jkdowdle dismissed stale reviews from sternetj and aecorredor via 466e963 February 6, 2024 19:27
@jkdowdle jkdowdle changed the title feat: allow for providing safe area provider props and react-navigation theme feat: allow for providing safe area provider props Feb 6, 2024
@jkdowdle
Copy link
Contributor Author

jkdowdle commented Feb 6, 2024

@aecorredor @sternetj sorry for the force push, but I think the change set will now be more straight forward. Just the safe area props, nothing with the navigation

@jkdowdle jkdowdle merged commit d44a25f into master Feb 6, 2024
3 checks passed
@jkdowdle jkdowdle deleted the extra-props branch February 6, 2024 19:41
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.

4 participants