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

Add css var for header webkit backdrop filter #20473

Merged

Conversation

bgoncal
Copy link
Member

@bgoncal bgoncal commented Apr 8, 2024

Breaking change

Proposed change

This adds a CSS variable to customize -webkit-backdrop-filter for header.

Screen_Recording_2024-03-10_at_16.56.26.mov

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

You only implemented this for the Lovelace dashboard now, we should probably do it more general?

src/layouts/home-assistant-main.ts Outdated Show resolved Hide resolved
src/panels/lovelace/hui-root.ts Outdated Show resolved Hide resolved
@bgoncal
Copy link
Member Author

bgoncal commented Apr 8, 2024

You only implemented this for the Lovelace dashboard now, we should probably do it more general?

Do I need to replicate the change in many places or there is a way to declare this variable once and apply to all headers?

@bramkragten
Copy link
Member

bramkragten commented Apr 8, 2024

Most use ha-top-app-bar/ha-top-app-bar-fixed, so implementing it there will get most cases.

@bgoncal bgoncal force-pushed the header-webkit-backdrop-css-var-2 branch from 3bc14ca to 9a3782a Compare April 8, 2024 14:56
@github-actions github-actions bot added the Supervisor Related to the supervisor panel label Apr 8, 2024
@bgoncal bgoncal force-pushed the header-webkit-backdrop-css-var-2 branch from 9a3782a to 91a9458 Compare April 8, 2024 14:58
@bgoncal
Copy link
Member Author

bgoncal commented Apr 8, 2024

Most use ha-top-app-bar/ha-top-app-bar-fixed, so implementing it there will get most cases.

Ok, I've added to those + some places I thought were relevant, maybe they are not needed to those general ha-top-app-bar-*

@bgoncal bgoncal requested a review from bramkragten April 9, 2024 07:54
@@ -27,6 +27,9 @@ export class HaTopAppBarFixed extends TopAppBarFixedBase {
padding-inline-start: 20px;
padding-inline-end: initial;
}
.header {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there is an element that has the header class? Have you tested this?

Copy link
Contributor

@Nezz Nezz Apr 9, 2024

Choose a reason for hiding this comment

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

I think this is where this should go (and it was already done in the second commit):


I think this and the one in ha-top-app-bar.ts should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is an element that has the header class? Have you tested this?

Oh sorry, I got confused here, if I want to add it directly to ha-top-app-bar-fixed can I just remove ".header"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2024-04-09 at 15 13 14 I think I am adding it twice

Copy link
Member Author

Choose a reason for hiding this comment

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

@Nezz but would it then go to all headers in HA?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, that way it'd only get added to the dashboards. Add it under this one:


and this one:
.mdc-top-app-bar {

@bgoncal bgoncal force-pushed the header-webkit-backdrop-css-var-2 branch from f75c5da to 38f8b4e Compare April 9, 2024 13:14
Comment on lines 911 to 912
-webkit-backdrop-filter: var(--app-header-backdrop-filter, unset);
backdrop-filter: var(--app-header-backdrop-filter, unset);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-webkit-backdrop-filter: var(--app-header-backdrop-filter, unset);
backdrop-filter: var(--app-header-backdrop-filter, unset);
-webkit-backdrop-filter: var(--app-header-backdrop-filter, none);
backdrop-filter: var(--app-header-backdrop-filter, none);

I believe the default value should be none, not unset

Copy link
Contributor

Choose a reason for hiding this comment

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

Also note that both properties should be added everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I first need to know where exactly they need to be and where they don't

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed from other places and kept it in these 3 files you see in the changes, what do you think?

@bgoncal bgoncal force-pushed the header-webkit-backdrop-css-var-2 branch from 38f8b4e to edd96db Compare April 9, 2024 13:31
@github-actions github-actions bot removed the Supervisor Related to the supervisor panel label Apr 9, 2024
@Nezz
Copy link
Contributor

Nezz commented Apr 9, 2024

You only implemented this for the Lovelace dashboard now, we should probably do it more general?

After having a look around, it seems that only these two panels support transparency in their headers:
https://github.com/search?q=repo%3Ahome-assistant%2Ffrontend+calc%28var%28--header-height%29+%2B+env%28safe-area-inset-top%29%29%3B&type=code

The rest of the views (logbook, settings, notifications, etc) don't allow the content to slide under the header. Therefore adding the backdrop-filter there would be just slow down the rendering.

@bgoncal
Copy link
Member Author

bgoncal commented Apr 9, 2024

You only implemented this for the Lovelace dashboard now, we should probably do it more general?

After having a look around, it seems that only these two panels support transparency in their headers: https://github.com/search?q=repo%3Ahome-assistant%2Ffrontend+calc%28var%28--header-height%29+%2B+env%28safe-area-inset-top%29%29%3B&type=code

The rest of the views (logbook, settings, notifications, etc) don't allow the content to slide under the header. Therefore adding the backdrop-filter there would be just slow down the rendering.

Screenshot 2024-04-09 at 15 52 00 Developer tools header too

@Nezz
Copy link
Contributor

Nezz commented Apr 9, 2024

Nice, I haven't found the developer tools. For future reference that's styled here for offset content:

padding-top: calc(
var(--header-height) + 48px + env(safe-area-inset-top)
);

@bramkragten bramkragten merged commit 7556ab9 into home-assistant:dev Apr 10, 2024
13 checks passed
@bgoncal bgoncal deleted the header-webkit-backdrop-css-var-2 branch April 10, 2024 09:41
@nayzm
Copy link

nayzm commented May 3, 2024

Thank you for working on this.

Just wondering would it be possible to implement some sort of support for kiosk-mode? On mobile devices with app-header hidden, the background doesn't stretch underneath the status icons on iOS, so it just shows a solid colour.

@bgoncal
Copy link
Member Author

bgoncal commented May 3, 2024

Thank you for working on this.

Just wondering would it be possible to implement some sort of support for kiosk-mode? On mobile devices with app-header hidden, the background doesn't stretch underneath the status icons on iOS, so it just shows a solid colour.

You can workaround this by setting a app-header-background-color right? This would change the color of the status bar too

@nayzm
Copy link

nayzm commented May 3, 2024

Thank you for working on this.
Just wondering would it be possible to implement some sort of support for kiosk-mode? On mobile devices with app-header hidden, the background doesn't stretch underneath the status icons on iOS, so it just shows a solid colour.

You can workaround this by setting a app-header-background-color right? This would change the color of the status bar too

It doesn't unfortunatly, even when setting a rgba for app-header-background-color.

4F012F27-A142-4AAF-8D80-0C3E056299DE_4_5005_c

@bgoncal
Copy link
Member Author

bgoncal commented May 3, 2024

Keep an eye on this PR, it may help dealing with this: home-assistant/iOS#2757

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