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

refactor: new way for handling insets #207

Merged
merged 4 commits into from
Aug 18, 2023
Merged

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Aug 17, 2023

📜 Description

Setup onApplyWindowInsetsListener that manages systemBars and navigationBar on rootView to get correct and constant insets.

💡 Motivation and Context

If we setup onApplyWindowInsetsListener on EdgeToEdgeReactViewGroup, then sometimes insets are including keyboard size (it's is 0, 1225 (top, bottom) instead of expected 144, 84).

I discovered that if we setup onApplyWindowInsetsListener on rootVIew - in this case insets are always stable. So I reworked this part (worth to note that onApplyWindowInsetsListener that detects keyboard size changes still should listen to EdgeToEdgeReactViewGroup because otherwise it's not working).

It fixes a problem with content jump, however the old approach (setting padding for action_bar_root) is not working anymore - instead we should apply margin. Last, but not least - we should setup it asynchronously, because if we setup it in onAttachedToWindow/init then it'll add undesired padding:

image

Area in blue - undesired margin

Fixes #51

📢 Changelog

Android

  • apply margin to action_bar_root instead of padding;
  • for managing paddings setup onApplyWindowInsetsListener on rootView, keep onApplyWindowInsetsListener on EdgeToEdgeView just for detecting keyboard size changes;
  • setup onApplyWindowInsetsListener on rootView asynchronously (to avoid unnecessary margin on mount);
  • added ThemedReactContext extension that returns rootView;

🤔 How Has This Been Tested?

Tested (fabric/paper) on:

  • Pixel 7 Pro (Android 13);
  • Redmi note 5 Pro (Android 9);

📸 Screenshots (if appropriate):

Before After
telegram-cloud-photo-size-2-5402402906965133775-y telegram-cloud-photo-size-2-5402402906965133774-y

📝 Checklist

  • CI successfully passed

@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific refactor You changed the code but it didn't affect functionality labels Aug 17, 2023
@kirillzyusko kirillzyusko self-assigned this Aug 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 17, 2023

📊 Package size report

Current size Target Size Difference
60907 bytes 60759 bytes 148 bytes 📈

@kirillzyusko kirillzyusko merged commit dfe353d into main Aug 18, 2023
@kirillzyusko kirillzyusko deleted the refactor/insets-handler branch August 18, 2023 11:25
@thespacemanatee
Copy link
Contributor

I don't think we should set the margin here because it will push all the content down which kind of defeats the purpose of setting the status bar to translucent, but correct me if I'm wrong because I didn't really investigate this. I believe this is causing a regression where all my content is being pushed below the status bar even when set to translucent

@kirillzyusko
Copy link
Owner Author

@thespacemanatee yes, most likely this PR caused your regression. Let's move all our conversation to the issue that you opened 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🐛 bug Something isn't working refactor You changed the code but it didn't affect functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RNKC-053] - native-stack + statusBarTranslucent causes incorrect positioning
2 participants