Skip to content

Fixes the rendering of the weather card#2318

Closed
alex3305 wants to merge 1 commit into
home-assistant:devfrom
alex3305:fix-rendering-weather-card
Closed

Fixes the rendering of the weather card#2318
alex3305 wants to merge 1 commit into
home-assistant:devfrom
alex3305:fix-rendering-weather-card

Conversation

@alex3305
Copy link
Copy Markdown
Contributor

This was reported in #2294 and caused by an undefined variable. This variable is removed with this commit.

@zsarnett
Copy link
Copy Markdown
Contributor

We do not need to remove this variable. We need to correctly pass it to the weather legacy card

@alex3305
Copy link
Copy Markdown
Contributor Author

@zsarnett Any suggestion on where that is done? Because I would like to ammend this PR if possible. Otherwise it can be closed.

@zsarnett
Copy link
Copy Markdown
Contributor

If you look at the weather card in the Lovelace Cards directory it uses legacy wrapper. The legacy wrapper need to correctly pass config or we need to up the legacy weather card to use the config it's given and set this.config correctly. I havent drilled into it yet. But that's the basis

@zsarnett
Copy link
Copy Markdown
Contributor

Also this is an issue for the plant card as well. We can fix both as it should be the same issue

An undefined variable was referenced. This commit added a new config object
which is normally just provided and fixes the rendering.
@alex3305
Copy link
Copy Markdown
Contributor Author

I've ammended my commit with the same property as is defined by the plant card. I didn't see your new message unfortunately. So I am unsure if this is the right approach.

@zsarnett
Copy link
Copy Markdown
Contributor

Have you tested this approach?

@alex3305
Copy link
Copy Markdown
Contributor Author

Yes, I've tested this in my local dev environment with a pip3 install and --skip-pip when starting up Home Assistant and the card rendered again. I've also tested it with the Home Assistant provided frontend and there the error still occurs.

@zsarnett
Copy link
Copy Markdown
Contributor

And can your try setting a name variable

@alex3305
Copy link
Copy Markdown
Contributor Author

@zsarnett I'm unsure what you mean with:

And can your try setting a name variable

@zsarnett
Copy link
Copy Markdown
Contributor

zsarnett commented Dec 14, 2018

name: test in the love lace config for the card

@zsarnett
Copy link
Copy Markdown
Contributor

Or is this an issue for states UI?

@alex3305
Copy link
Copy Markdown
Contributor Author

alex3305 commented Dec 14, 2018

This is an issue for the states-ui. I'm also unable to open lovelace on my dev environment, because it's triggering home-assistant/core#18193.

Edit: Appearently it loaded after some retries. No specific lovelace errors given, but also no name displayed in either weather cards (states and lovelace).

@balloob
Copy link
Copy Markdown
Member

balloob commented Dec 16, 2018

This won't fix it … this.config is still not defined just because you define it as a property. We need to add a guard to it.

@alex3305 alex3305 closed this Dec 16, 2018
@ghost ghost removed the in progress label Dec 16, 2018
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants