Skip to content

Dont change config on init#2044

Merged
zsarnett merged 10 commits intohome-assistant:devfrom
bramkragten:dont-change-config-on-init
Nov 22, 2018
Merged

Dont change config on init#2044
zsarnett merged 10 commits intohome-assistant:devfrom
bramkragten:dont-change-config-on-init

Conversation

@bramkragten
Copy link
Copy Markdown
Member

Prevent an edit of the config on init and allows to open the raw yaml

zsarnett
zsarnett previously approved these changes Nov 12, 2018
}

private _changed(ev): void {
if (!this.hass || this._initializing) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How is it possible that these things are fired before we got rendered? Is that on initialization?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah apparently...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So is this PR solving problems or fighting symptoms? 😉

In Polymer versions, we would always check if the value was undefined and ignore that one. I wonder if we can do something similar, because adding initialized logic to each place we use a dropdown won't scale.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, instead of using ev.target.value, we can also inspect ev.detail.value, I wonder if it's different.

@zsarnett zsarnett dismissed their stale review November 17, 2018 17:10

Changes were made after review


protected async firstUpdated(): Promise<void> {
if (this._initializing) {
await this.updateComplete;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bit surprised about this, isn't this method called after the first time has been rendered, so awaiting this promise means you wait for 2nd render?

@bramkragten bramkragten self-assigned this Nov 20, 2018
@zsarnett zsarnett merged commit a825613 into home-assistant:dev Nov 22, 2018
@ghost ghost removed the in progress label Nov 22, 2018
@bramkragten bramkragten deleted the dont-change-config-on-init branch November 22, 2018 07:59
@bramkragten bramkragten added this to the 0.83 milestone Nov 22, 2018
balloob pushed a commit that referenced this pull request Nov 26, 2018
* dont change config on init

* set default title empty

* used firstUpdated instead of updated

* prevent double events

* check if val changed

* typing

* clean

* lint

* clean

* prettier is having a fight
@balloob balloob mentioned this pull request Dec 5, 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.

5 participants