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

Margin mode #85

Merged
merged 6 commits into from May 28, 2020
Merged

Margin mode #85

merged 6 commits into from May 28, 2020

Conversation

jacobp100
Copy link
Collaborator

@jacobp100 jacobp100 commented May 25, 2020

#50 Summary

Adding margin mode to safe area view

I noticed while doing this that a view's safe area is impacted by the position of itself. If you set a margin, the safe area gets decreased, so you get an infinite loop.

To work around this, I use the parent view controller's view's safe area (i.e. a modal, stack navigation controller, or root view controller). I removed the logic from Android that mimicked iOS's behaviour so they're now hopefully the same.

I'm hoping this change will make it match closer to v1 SafeAreaView, but I'm not sure if this will cause other issues - what do you think?

Note - I looked briefly border modes too - but they use floats instead of yoga values, so made it much more complicated to implement

Test Plan

We have a new playground

Simulator Screen Shot - iPhone 11 - 2020-05-26 at 18 36 00

@janicduplessis
Copy link
Collaborator

I think it would be nice to make android apply insets the same way as ios when it is not touching edges. But that's for another time :)

Is this ready to review?

@jacobp100 jacobp100 changed the title WIP: margin mode Margin mode May 26, 2020
@jacobp100
Copy link
Collaborator Author

Just pushed the Android implementation and updated the PR description

@jacobp100
Copy link
Collaborator Author

If we go this route, I just realised the fallback on iOS 10 would need updating to just use the parent view controller's insets. Will update tomorrow

Copy link
Collaborator

@janicduplessis janicduplessis left a comment

Choose a reason for hiding this comment

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

Awesome, let's go with this. I've been testing this with react navigation and it fixes the lag issues. Got a few minor tweaks but I'll just address in a follow up.

@janicduplessis janicduplessis merged commit 778e3eb into th3rdwave:master May 28, 2020
@jacobp100 jacobp100 deleted the margin-mode branch May 29, 2020 14:28
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.

None yet

2 participants