-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(android): Adding native margin handling #8268
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
base: main
Are you sure you want to change the base?
Conversation
|
@theproducer think it would be beneficial to implement a mechanism for Dynamic Update (Theme Changes). We should continue using something similar to handleOnConfigurationChanged to automatically update the bar styles (like status-bar plugin) when the user changes the system theme (dark/light) without requiring an app restart. |
|
I don't understand this approach. Why would anyone want to use the I have not tested it, but by the looks of it I think Also this approach does not allow for what I think is the most straight forward and non-breaking solution: using padding in broken webviews, and native envs for non-broken webviews. I really suggest you take a look at the most recent source code of the |
| Insets safeArea = insets.getInsets(WindowInsetsCompat.Type.systemBars() | WindowInsetsCompat.Type.displayCutout()); | ||
| Insets imeInsets = insets.getInsets(WindowInsetsCompat.Type.ime()); | ||
| boolean keyboardVisible = insets.isVisible(WindowInsetsCompat.Type.ime()); | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM && useCSSVariables) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM check in place to only apply this for apps that are edge-to-edge? But there could be Capacitor apps with the edge-to-edge optout (as it was suggested for status-bar plugin around Cap 7 time), in which case it would enter this if condition, when perhaps it shouldn't?
Also what about if a developer enables edge-to-edge on the activity explicitly, in which case they'd have edge-to-edge on versions lower than 15?
Or maybe I'm missing some context here on why this version check is in place.
| initSystemBars(); | ||
| } | ||
|
|
||
| private boolean hasFixedWebView() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this check not needed anymore? Not clear to me.
Thanks for the feedback — quick clarification on the underlying premise here. We did explore only injecting vars for “broken” WebViews and relying on native behavior for “fixed” ones, and we’ve spoken directly with the Google WebView team about From a DX standpoint for Capacitor, we’re intentionally standardizing on a single web-facing model. iOS teams already design against CSS safe-area variables, and we want that same contract on Android rather than switching between native padding vs CSS vars based on WebView state. So the decision to always inject CSS variables is deliberate for consistency and reliability across platforms. |
What's your source for that? To my knowledge, pretty much every distro and its webview implementation is using Chromium. So while you might theoretically be correct, I don't think it makes sense to account for like < 1% (probably even less) of devices. (you might even be able to further solve this by checking if the webview implementation being used actually is chromium) Moreover, CSS var injection is inherently a broken strategy. So it still is not a solution for "consistency and reliability across platforms". Frameworks that use env variables will break. Retrieved insets can still be wrong. You're not accounting for the
This makes total sense, but that actually advocates for not doing it your way, but instead doing it with native env variables. Because with the native env strategy, a developer would be able to drop in his webapp code into Capacitor and it just works. With your strategy however, he first has to make adjustments to make it work. |
You mean |
|
Thanks for the thoughtful feedback — we genuinely appreciate the deep technical review here.
In real-world testing, we’ve encountered devices where the version should be “fixed” but
We do account for the viewport tag and explicitly normalize behavior with iOS. If Also worth noting: the Android team themselves recommend applying insets either via the container or via injected padding when the WebView owns the content, rather than assuming native handling will always be sufficient. Our approach follows that guidance for the “WebView owns the content” case:
This is where the product philosophy differs a bit. In practice, modern web apps already account for safe areas and mobile layout constraints — especially since iPhone X. For Capacitor, we’re intentionally normalizing that existing web pattern (CSS-driven safe-area handling) across both platforms. That adjustment is already part of most real-world responsive design systems; we’re just making the contract consistent. We do provide a configuration to disable this behavior if developers prefer an alternative approach (e.g. using the Capacitor Community Safe Area plugin), which we believe strikes the right balance from a DX perspective. Now for the PR itself...it may have valid bugs, which we are currently working through. Please let us know if you find any and we'll fix them before merging. And again — thanks for taking the time to dig into this and share detailed feedback. |
I'm very curious in what setups you've seen this behavior. Would you be willing to share that? I've tested across tons of devices with different setups, and I've yet to come across one that doesn't work reliably |
I think there might be a misunderstanding. Because that's exactly the same as what I am saying. But with the native env approach, this is actually 100% true. While with the var injection approach, this is like 85% true. Because with the latter a developer should switch from using envs to vars. And if he's using a plugin or anything that uses envs, he should fork that and make it use vars (or make a FR, but that's unlikely to be accepted I think)
This is only partially true. It checks for
True, I saw that too. I think it's weird that they published that blogpost. It might have been a misunderstanding or miscommunication between the teams. It would be a wise thing to request some clarification/rectification around this. Most important thing is that this blogpost was released before they ended up fixing the safe area issues in the webview. So it's kinda outdated anyways |
Hmm, I'll look into this as well, thanks!
I'll see what I can get. These reports are coming from some of our internal users (who are depending on those custom css variables being there). Again, thanks for the feedback! If you are on the Ionic Discord, I'm available there too if you want to have a more real time discussion on some of these issues. |
This PR adds native view margin handling as suggested in comments from the initial PR (Thanks to @tafelnl for the suggestions!).
Additionally, as a result of some internal testing, we've decided to always inject the "correct" safe area inset css variables, regardless of the system webview version (configurable via updated plugin settings).