Skip to content

Conversation

@Gregman-js
Copy link

@Gregman-js Gregman-js commented Jan 25, 2025

Summary

Enable EdgeToEdge on WebViewActivity, show WebView under navigation bar.

serverManager.getServer()?.version?.isAtLeast(2024, 1)

Applying insets depend on server release with this changes. We shouldn't display webview under navigation bar before frontend release because some ui elements will overlap with navigation bar.
After releasing this two PR we can even show webview under status bar on Android and iOS in future PR's.
We need to do something in this area because when HA will target api 35 some apis releated to changing status bar and navigation bar color will be not available docs

Screenshots

Left iOS, Right: Android
image

Any other notes

Dependency: home-assistant/frontend#23811
Related issue: #4547

Comment on lines +267 to +277
val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars())
val bottomInset = if (bottomInsetsApplied) 0 else insets.bottom

v.updatePadding(
top = insets.top,
bottom = bottomInset
)

rootInsets = Insets.of(insets.left, 0, insets.right, insets.bottom - bottomInset)

WindowInsetsCompat.CONSUMED
Copy link
Member

Choose a reason for hiding this comment

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

Could make sense to call the default onApplyWindowInsets here so the env is set properly in case the bug is ever fixed in Android

@jpelgrom
Copy link
Member

I don't want to do too much review while there is no frontend decision, but why is the status bar/top inset excluded?

@Gregman-js
Copy link
Author

@jpelgrom From my initial analysis, it turned out that the frontend is not yet adapted to the top insets. Even the iOS app does not draw webview under the status bar(If I haven't missed anything)

@MindFreeze
Copy link
Member

I don't want to do too much review while there is no frontend decision

We have decided to accept the overall approach. Just have some nitpicks on the frontend PR

From my initial analysis, it turned out that the frontend is not yet adapted to the top insets

I think the app should provide all the insets and let frontend decide what to do. As long as we are not breaking it, of course


binding = ActivityWebviewBinding.inflate(layoutInflater)

bottomInsetsApplied = serverManager.getServer()?.version?.isAtLeast(2026, 1) == true
Copy link
Member

Choose a reason for hiding this comment

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

The server/server version might change after the activity is created, most likely in multi-server scenarios. This will only check the current active server which might break UI if at some point it loads another, older frontend version. Not sure what the best fix is but that scenario should be handled.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, sorry for the delay. I will check what we can do with this.

@Gregman-js
Copy link
Author

I saw such an article recently https://developer.chrome.com/blog/edge-to-edge

Apparently chrome 135 will support EdgeToEdge and CSS env variables. I don't know yet how it will relate to webview

@MindFreeze
Copy link
Member

Frontend PR merged. It won't do anything until Android sets the vars but you can test with nightly now

@TimoPtr
Copy link
Member

TimoPtr commented May 21, 2025

This PR is needed for #5328

@TimoPtr
Copy link
Member

TimoPtr commented May 22, 2025

@Gregman-js what needs to be done here ?

Also if you could review this PR #5345 it would be of a great help :)

@Gregman-js
Copy link
Author

I will rebase and update this PR.
Do you have any idea how to resolve this comment? #5010 (comment)
I am not sure how to approach this problem.

@TimoPtr
Copy link
Member

TimoPtr commented May 22, 2025

I'm checking your PR currently. I think there are something still missing on the frontend to properly handle the top bar and other screens. I'm going to open a branch with your changes on the latest version of main with my changes. I'll try to handle also the case of the server changing.
image

@Gregman-js
Copy link
Author

Gregman-js commented May 22, 2025

Yes, you are right. We can't render WebView under the status bar because frontend does not support it yet. My frontend PR only adds possibility for it. We have to add top padding (I think this PR does it) or some block above WebView

@jpelgrom
Copy link
Member

Closing in favor of #5346

@jpelgrom jpelgrom closed this May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants