-
Notifications
You must be signed in to change notification settings - Fork 315
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
Use same variable for status bar color as frontend #2757
Conversation
The purpose of this view is to extend the background of the top header so it looks seamless - like a navigation controller. Is the header background color always the theme color or is this going to introduce discrepancies that look bad? What does this look like in the default themes? |
Indeed, but browsers also do this "trick" (#2752), but for browsers the frontend uses app-theme-color instead of the app-header, so it makes sense to align I guess. @bramkragten can you confirm this? For the default themes it looks the same with or without this change |
Yes, it'd align this way. It does not make a difference for the default theme, but it does for ones that use transparency for I believe @bgoncal uses a similar theme too since he implemented home-assistant/frontend#20473 |
@@ -6,6 +6,7 @@ public struct ThemeColors: Codable { | |||
public enum Color: String, CaseIterable { | |||
// these are in WebSocketBridge.js in this repo (we inject it) | |||
case appHeaderBackgroundColor = "--app-header-background-color" |
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.
I wonder if we should fall back to this if the theme color is not set?
I checked with front end and app-theme-color will be always set, so no need to fallback |
Will this also resolve this issue? |
No, that's a separate issue. I'll try to get it fixed. |
Great, really looking forward to it! What's the ETA from a fix like this to a release on iOS? |
If it gets merged before the last week of the month it probably will be in the month's release, otherwise the month after. (Unless it is something critical) |
Sounds great, if I join the Testflight beta, will things like this get released sooner in the beta? |
Yes |
That issue/feature looks like a frontend change to me. You might not even need an app update. |
Aha, ok, do I need a Home Assistant image beta version then? Or how do you mean this gets fixed? |
Got the TestFlight update which pushed today, but the fix isn't there right? Because I still have the same issue as before? Thank you! |
Nope, the PR got reverted because the front end font use "app-theme-color" with the same color as the top navigation bar all the times, so it looks weird in the app. |
Is |
@jockebq |
Ah, that I have already set. What I am talking about is this issue: home-assistant/frontend#18277 |
Ok, I got it now, this PR was never meant to fix that, this was the original issue: #2752 |
Wasn't this suppose to fix the issue? |
This was reverted in #2768:
(see #2752) The frontend was fixed to provide the correct color variables in Home Assistant 2024.6 (home-assistant/frontend#20758). I guess now the fix can be re-applied @bgoncal? I'm not sure how quickly people update. |
Summary
We were using
--app-header-background-color
for status bar in the app and frontend uses--app-theme-color
, this PR changes that to keep consistency with frontendScreenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes