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

Fix visual differences between regular and energy dashboards #20654

Merged
merged 5 commits into from
Apr 30, 2024

Conversation

Nezz
Copy link
Contributor

@Nezz Nezz commented Apr 28, 2024

Proposed change

Fix visual differences between regular and energy dashboards. This is an improvement for the blurring introduced in #20473 which will go live in 2024.5.

The energy dashboard is structured like this:

<hui-view id="view" />

Wheres lovelace is:

<div id="view">
  <hui-view />
</div>

This results in the background getting applied differently on the two kinds of dashboards.

In the unscrolled state lovelace looked like this:

With this fix, the background will extend on lovelace behind the header, matching the energy dashboard:

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

app-header-backdrop-filter: blur(20px)
app-header-background-color: rgba(128, 128, 128, 0.3)
background-image: "center / cover no-repeat fixed url('something.jpg')"

Additional information

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:

@Nezz Nezz marked this pull request as ready for review April 28, 2024 19:14
@Nezz Nezz force-pushed the fix/DashboardBackground branch from 5bcda93 to 8fef80d Compare April 28, 2024 19:17
@Nezz
Copy link
Contributor Author

Nezz commented Apr 30, 2024

@bramkragten would it be possible to get this into 2024.5?

@piitaya
Copy link
Member

piitaya commented Apr 30, 2024

@Nezz Can we also use the same structure as regular dashboard for energy dashboard?

<hui-view
  id="view"
  .hass=${this.hass}
  .narrow=${this.narrow}
  .lovelace=${this._lovelace}
  .index=${this._viewIndex}
  @reload-energy-panel=${this._reloadView}
></hui-view>

becomes

<div id="view" @reload-energy-panel=${this._reloadView}>
  <hui-view
    .hass=${this.hass}
    .narrow=${this.narrow}
    .lovelace=${this._lovelace}
    .index=${this._viewIndex}
  ></hui-view>
</div>

@Nezz
Copy link
Contributor Author

Nezz commented Apr 30, 2024

Sure, I'll update this branch to see how that looks

@piitaya piitaya added this to the 2024.5 milestone Apr 30, 2024
@Nezz
Copy link
Contributor Author

Nezz commented Apr 30, 2024

The CSS change is still needed to ensure that the image is on the same element as the padding, but otherwise it should be good to go.

@piitaya piitaya merged commit 334c245 into home-assistant:dev Apr 30, 2024
13 checks passed
@Nezz Nezz deleted the fix/DashboardBackground branch April 30, 2024 13:15
Comment on lines -1016 to -1017
}
hui-view {
Copy link
Member

@bramkragten bramkragten May 1, 2024

Choose a reason for hiding this comment

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

This should be reverted I think.

The issue is that the background can be set with a theme variable --lovelace-background that can set on the view or with a config variable in YAML. Putting the background option a level higher means that the view theme can not set the background of the view, because it can not target anything above itself.

You can set the background on both #view and hui-view, but that will not give the best experience either when using something different than a solid color.

Copy link
Member

@bramkragten bramkragten May 1, 2024

Choose a reason for hiding this comment

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

Or, a bit ugly... #20683

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.

3 participants