search

croma-app / huehive-mobile-app

[HueHive] : An open source react native app for web, iOS and android for color palette management
https://play.google.com/store/apps/details?id=app.croma
MIT License
60 stars 21 forks source link

Show header navigation in its entiety on Safari #106

Closed ashokaditya closed 3 years ago

ashokaditya commented 3 years ago

fixes #65

Screenshots (Safari):

Screenshot 2020-10-19 at 20 12 18 Screenshot 2020-10-19 at 20 11 58 Screenshot 2020-10-19 at 20 14 46
ashokaditya commented 3 years ago

@bhujoshi please add a hacktoberfest-accepted label to this if all good. Or add hacktoberfest as a topic to your repo. 🙏

https://www.youtube.com/watch?v=FxBv4FMcDgo&ab_channel=DigitalOcean

kamalkishor1991 commented 3 years ago

@ashokaditya thanks a lot for sending a fix. I added hacktoberfest as a topic. Can you please add more details on what is the issue and why do we have to add it to all the screen components? Seems really unmaintainable to add it to all the components right?

ashokaditya commented 3 years ago

@kamalkishor1991 thanks for following up. So as I see it the Safari bug is due to the fact that there's a max-width set on top of the app content at the View node which is shared for native and web apps. The rest of the app is configured via React Navigation that have components mapped to corresponding route names. If this was not the case I could have encapsulated these components into a top component and added the styles to only that. But doing that would be a big change.

So I went with a smaller change. The idea was to then render the app content in its full width (which is within the top View) and add a max-width restriction to the content that renders below the header. Mot using React Navigation and writing a custom navigation component would be a better approach in my opinion. But this is also a big change. Also, I thought this should give you some idea to improve on it later. But for now, at least Safari would render much the same way as other browsers.

kamalkishor1991 commented 3 years ago

@ashokaditya thanks for the explanation, I still don't understand why it is only a problem with safari. Can you send me an email and I can add you to the slack group to discuss this more? email: kkjoshi.it@gmail.com

ashokaditya commented 3 years ago

@kamalkishor1991 thanks for asking the relevant questions which led me to look into it bit closer. I found a simpler more acceptable fix after all. At least compared to the previous one. Essentially React Navigation adds styles that Safari renders differently than Chrome or Firefox. This fix overrides the react navigation style, particularly overflow so that the header is visible in its entirety.

kamalkishor1991 commented 3 years ago

Looks good. Thanks

ashokaditya commented 3 years ago

👍 thanks @kamalkishor1991, would you also please add a hacktoberfest-accepted label to this PR. 🙏