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

View background settings #23133

Merged
merged 23 commits into from
Dec 22, 2024

Conversation

silamon
Copy link
Contributor

@silamon silamon commented Dec 4, 2024

Proposed change

Implements

  • tiled background (previously done in Support tiled view background in UI #22643 and continued upon)
  • transparancy for background
  • size settings for background
  • alignment settings for background
  • scrolling background or fixed background

Fixes #22512
Fixes #22931
Fixes #22511

Help wanted: Testing in cast specific views and iOS testing.

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

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:

@silamon silamon added Documentation Needed Needs UX Pull requests requiring a review from the Home Assistant design team labels Dec 4, 2024
@github-actions github-actions bot added the Cast Related to Home Assistant Cast UI label Dec 4, 2024
@marcinbauer85 marcinbauer85 requested a review from piitaya December 5, 2024 08:45
@silamon silamon removed the Needs UX Pull requests requiring a review from the Home Assistant design team label Dec 10, 2024
}

static get styles(): CSSResultGroup {
return css`
:host {
display: relative;
}
/* Fixed background hack for Safari iOS */
Copy link
Member

Choose a reason for hiding this comment

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

Since your removed this, does it still work on iOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was introduced in #22531, I've added it back but can't test on iOS.

@bramkragten
Copy link
Member

The background now scrolls with the page, previously it was static and would not scroll. We can make this an option too, but we should at least make sure the old behaviour stays by default.

@bramkragten
Copy link
Member

Can we add fixed as option in the UI too?

@marcinbauer85
Copy link
Contributor

@silamon I've modified the initial design to add Background attachment via UI and a <ha-button-toggle-group>

  • Scroll (default)
  • Fixed

@silamon
Copy link
Contributor Author

silamon commented Dec 17, 2024

@silamon I've modified the initial design to add Background attachment via UI and a <ha-button-toggle-group>

  • Scroll (default)
  • Fixed

Do you have a link somewhere? I can't seem to find a design that got updated.
EDIT: Nevermind, found it.

@silamon silamon added the Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Dec 18, 2024
private _computeBackgroundProperty(
background?: string | LovelaceViewBackgroundConfig
) {
if (typeof background === "object" && background.image) {
Copy link
Member

@bramkragten bramkragten Dec 20, 2024

Choose a reason for hiding this comment

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

If background is an object, can we set the css rules individually instead of using the shorthand format?

const viewBackgroundOpacity = this._computeBackgroundOpacityProperty(
this.background
);
this.style.setProperty("--view-background-opacity", viewBackgroundOpacity);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we use variables for this? So it can also be used by themes? Then we should only set it if _computeBackgroundOpacityProperty doesn't return null right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main use is to be able to use it in the css styling, like it was done for the background. For to be used in themes, there's a different theming variable used in the fallback:

var(--view-background, var(--lovelace-background, var(--primary-background-color)));

@bramkragten bramkragten force-pushed the tiled-background-continue branch from 59bc691 to 9f9179a Compare December 22, 2024 16:14
@bramkragten bramkragten enabled auto-merge (squash) December 22, 2024 16:14
@bramkragten bramkragten merged commit 7900eb4 into home-assistant:dev Dec 22, 2024
11 checks passed
@marcinbauer85
Copy link
Contributor

@silamon could you fix the background Size and Repeat to be a dropdown as opposed to being a radio group now?
Also please align the background attachment to be as on the design since now the whole UI uses 3 components, and takes a lot of space.
👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cast Related to Home Assistant Cast UI cla-signed Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants