Add dialog to save config#2100
Add dialog to save config#2100balloob merged 10 commits intohome-assistant:devfrom bramkragten:take-control
Conversation
| import { LovelaceCardConfig } from "../types"; | ||
| import { LovelaceConfig, LovelaceCardConfig } from "../types"; | ||
|
|
||
| export const migrateConfig = (hass: HomeAssistant): Promise<void> => |
There was a problem hiding this comment.
In a future PR, we should move this file to src/data/lovelace.ts, as that's also what we do for all other components.
| import { html, LitElement, PropertyDeclarations } from "@polymer/lit-element"; | ||
| import { fireEvent } from "../../../common/dom/fire_event"; | ||
| import { | ||
| showEditCardDialog, |
There was a problem hiding this comment.
This is perfect, we should do this for all dialogs.
| cardConfig: this.cardConfig, | ||
| reloadLovelace: () => fireEvent(this, "config-refresh"), | ||
| }); | ||
| showEditCardDialog(this, this.hass!, this.cardConfig!, () => |
There was a problem hiding this comment.
I think that this should take 2 arguments; this and an object with type EditCardDialogParams. It allows type checking and allows us to just store 1 object in the dialog class too. Example
There was a problem hiding this comment.
Oh, you already have this with EditDialogParams. Well then this function just needs an object of type EditDialogParams passed in .
|
|
||
| export const showEditCardDialog = ( | ||
| element: HTMLElement, | ||
| hass: HomeAssistant, |
There was a problem hiding this comment.
No need to pass hass, the dialog manager already takes care of it .
| this._cardConfig = cardConfig; | ||
| this._reloadLovelace = reloadLovelace; | ||
| public async showDialog(params: EditDialogParams): Promise<void> { | ||
| this._hass = params.hass; |
There was a problem hiding this comment.
I would just this._params = params
| }); | ||
|
|
||
| export class HuiDialogEditCard extends LitElement { | ||
| protected _hass?: HomeAssistant; |
There was a problem hiding this comment.
|
|
||
| export const showSaveDialog = ( | ||
| element: HTMLElement, | ||
| hass: HomeAssistant, |
There was a problem hiding this comment.
no need to pass hass, we should pass a single object to showSaveDialog
| cardConfig: this.cardConfig!, | ||
| reloadLovelace: () => fireEvent(this, "config-refresh"), | ||
| }; | ||
| showEditCardDialog(this, editCardDialogParams); |
There was a problem hiding this comment.
You should be able to just do showEditCardDialog(this, { … }). TypeScript will look at the shape of an object, not the "type" (if it's not a class)
| `; | ||
| if (!this._params) { | ||
| return html``; | ||
| } else { |
There was a problem hiding this comment.
No need for the else as you return inside the if, that will keep the function a bit more readable.
| if (!this._params) { | ||
| return html``; | ||
| } else { | ||
| return html` |
There was a problem hiding this comment.
Since the whole tag is just 1 turnery statement, I would split it up:
if (!this.params.cardConfig!.id) {
return html`
<hui-migrate-config…
`
}
balloob
left a comment
There was a problem hiding this comment.
few teeny tiny comments, ok to merge after that.
Relies on home-assistant/core#18655
Closes #2092