-
Notifications
You must be signed in to change notification settings - Fork 41
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
Supported edge-to-edge (Android) #841
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The idea is that need to match the height of the navigation bar (and tab bar) to the native height after the insets are applied. For example, with the navigation bar have to add height to match the status bar inset at the top. So going to add the insets to context so that they can be applied correctly on first render for subsequent scenes. For the first scene need something different because can't know the insets at first render - they're calculated dynamically on native side when view attached to window. For that will update the size natively
Only changed to BlendMode because got deprecation warning. So changing back now getting not available below api 29 warning
Set fistSystemWindows otherwise the insets aren't applied (as per https://medium.com/androiddevelopers/insets-handling-tips-for-android-15s-edge-to-edge-enforcement-872774e8839b). Original idea was to listen for applyWindowInsets and then update native size. But the base AppBarLayout is already listening and each View can only listen once (it doesn't even call onApplyWindowInsets if there's a listener). So listening in the toolbar view to avoid the conflict and then calling to the parent navigation bar. Need to do a synchronous measure and layout when insets change otherwise there's a noticeable UI jump - the toolbar renders behind sytem bar then jumps to proper unobscured position when the async measure and layout happens. Don't want to make the requestLayout call sync because who know what could happen - safest to have separate sync call when insets change
This ensures that every scene other than the first one has the correct values at initial render. Have to special-case the first scene because have to get these from the native render
Tried listening on the child toolbar view but when it's inside a collapsing toolbar the insets are consumed by it and so don't reach the toolbar. Instead listener to scene view - always reach here and can safely add as many listeners as necessary (tab bar will need to listen, for example)
The insets in navigation stack context will ensure the height is correct on first render for all scenes apart from the first. For the first have to get that dynamically on native side and then update the native height to match. There is a slight flicker because resizing in native is async. So the content jumps down as the navigation bar resizes. Will see if it's any better on the new arch
Was setting the layout params height when insets change - but this should be calculated automatically by android. Android does the layout params and React Native gets the style height - and these have to match. But removing the params height didn't work - Android set the height without the top inset. Fixed this by changing measure spec to unspecified. The onMeasure of AppBarLayout only adds the top inset if unspecified. Another thing. React Native had the wrong width when changing to landscape. The updateNodeSize doesn't have the landscaped width - it just has the new top inset. So changed the width to -1 and that seems to work. React Native sets the correct one
Was incorrectly setting the layout params height to include the top inset after the first scene. The layout params height shouldn't include the inset - Android adds the inset as padding and includes it in measured height. Now the layout params has the raw height - without inset - used it in the React Native size calc and just added the new top inset to it when the insets change
The getXInset are deprecated but targeting sdk 23 so have to use the deprecated. No point using both so removed the subsequent getInsets
Android studio used to insist on making them final if used in the runGuarded?! Doesn't mind any more, not sure why. But changing to match existing code
Not sure if this is clearer - need to call with unspecified for the inset to be included by AppBarLayout. But react native always calls with exactly so changing for consistency. The overridden onMeasure changes both this call and React Native's to unspecified
Must've forgot to enable when last upgraded
Turned off the measure unspecified on navigation bar when it contains collapsing bar - otherwise the collapsing bar doesn't get a height and android throws error. Could've set layoutparams height on the collapsing bar but prefer to only do unspecified when child is toolbar. Only need this for the coordinatorlayout scenario - on the home scene of twitter - without the unspecified the app bar doesn't have the right height. Think that's because needsCustomLayout is on so it's up to Android to size it rather than React Native. Also set status bar scrim to transparent so that the navigation bar content shows through - don't want status bar background color with edge-to-edge. Also set fitsSystemWindows on child view of collapsing so it draws behing the status bar. Again, makes sense for edge-to-edge
The top inset isn't added to the height of the navigation bar when there's a collapsing bar. Think because the collapsing bar has fitsSystemsWindows - but also it doesn't need it because the height for the navigation bar must be bigger than the normal height + top inset anyway
The first measure and layout after insets applied has to be synchronous else there's a flicker as the toolbar moves into place. This happens with or without collapsing toolbar. But with the collapsing toolbar the problem is trickier because the measure and layout has to happen after the collapsing toolbar's insets - it was happening after navigation bar's insets but before collapsing bar's insets. That's because the navigation bar requests layout and that's sync but that runs from the insets of navigation bar - and that's before collapsing bar's insets because it's parent first. Tried doing the same thing and listening to the inset of the scene but didn't work - because that just set up a boolean before collapsing insets applied and then the layout was requested when navigation bar insets were applied which was too early. Added a predraw listener in navigation bar and ran a sync measure and layout from there. That's late enough that all insets applied and early enough that there's no flicker
Copied approach from navigation bar. Gets the bottom inset. Didn't need to change the onMeasure to unspecified - think because the inset bottom is added as padding so is automatically included
Even though doing a sync measure and layout there was still a flicker when navigating to a scrollable navigation bar - tested navigating from tabs to tabs in twitter sample. Fixed by doing a sync measure and layout of the coordinatorlayout. This also works for the collapsingbar. So now either do sync on parent coordinatorlayout or, if there isn't one, sync on the navigationbar
When material 3 theme is used then context is a ThemeContextWrapper and not ReactContext. Copied the code from BottomAppBarView
On edge-to-edge the pinned toolbar collapses when using material3 theme. This is [an edge-to-edge bug with the collapsingtoolbar](material-components/material-components-android#2608) - it only shows up on material3 theme because it sets forceApplySystemWindowInsetTop to true. So turned this setting off - the setter isn't visible outside of material so get an 'error warning'. After fixing the scrim color doesn't show up when collapsing with material3 theme - the theme sets a value for scrimVisibilityHeightTrigger which means it doesn't use the height-based calculation anymore. Can't turn this off because not able to set negative value (the default if no theme value provided). So might offer a prop to set this in the future
Took ages to work out how make dialog edge-to-edge. Nothing worked. The key to it all was setting the windowIsFloating flag to false which can only be done from theme resx. After that the standard stuff worked - setting decorFitsSystemWindows to false and the status and navigation bars to transparent. Only did this if there are insets on main activity window because don't want edge-to-edge Dialog if edge-to-edge not enabled.
It's enough that onApply is called to indicate that edge-to-edge is on. If it's not on then onApply isn't called at all
Prefer to remove listeners on detached than when instance dropped - more in keeping with android to use built-in lifecycle. Also, onAttached runs before window insets so the order is good (even if it didn't the requestApplyInsets will make sure it works)
Added a predraw listener to ensure the any insets will have been applied. Then tinted status bar using windowinsetscontroller if edge to edge. Need to request apply insets so that if status bar added/removed the edge-to-edge detection runs before the predraw
Copied over the update size logic (cpp files etc) from action bar on fabric. Resizing ensures navigation bar insets are applied on first scene when first loaded
Copied over the update size logic (cpp files etc) from action bar on fabric. Resizing ensures tab bar insets are applied on first scene when first loaded
Added 32 for the top and bottom margins
Made framesize available on ios because component descriptor uses it - could've wrapped that bit in ifdef android but prefer to make framesize available - think it's clearer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #839
Changed the height of the
NavigationBar
when edge-to-edge is enabled to include the top inset. Similarly, the height of the primaryTabBar
has to include the bottom inset.In order to work out the top and bottom insets have to listen for
onApplyWindowInsets
. But doing that from theNavigationBarView
orTabNavigationView
would remove Android's own listener. So added listeners toSceneView
instead, high enough up to ensure the insets weren't consumed.Resized the native Views after the insets were applied. Because React Native doesn't synchronously resize there's a slight flicker. To limit the flicker to the first scene only, passed the insets to
NavigationStack
and stored them in React Context. Ensures theNavigationBar
is sized correctly on first render for subsequent scenes.To enable edge-to-edge call enableEdgeToEdge from within the MainActivity