Skip to content

Entities Card: Entity Row Editor#7134

Merged
bramkragten merged 26 commits intodevfrom
entity-editor
Sep 30, 2020
Merged

Entities Card: Entity Row Editor#7134
bramkragten merged 26 commits intodevfrom
entity-editor

Conversation

@zsarnett
Copy link
Copy Markdown
Contributor

Proposed change

image
image
image

Type of change

  • New feature (thank you!)

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request: TODO

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:

@zsarnett
Copy link
Copy Markdown
Contributor Author

8d3010cead6946c21a9a0a5200ee381f

Comment on lines +236 to +242
if (!this.value.type && "entity" in this.value) {
// @ts-ignore
const domain = this.value.entity.split(".", 1)[0];
rowType = `${
DOMAIN_TO_ELEMENT_TYPE![domain] ||
DOMAIN_TO_ELEMENT_TYPE!._domain_not_found
}-entity`;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If one of these types. I think I should set it to just be one a type of generic or something. BEcause it will see the type change and try and reload the same element again

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.

Uuuh whut?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is no type on the entity row for regular entities. The entity file comes from the Domain to Element Type. Well all of those domains will have the same editor file that needs loaded

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.

Oooh... can we make the check on the editor element instead of on the type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? We get the editor element from the type?

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.

Yeah, and all those types return the same element right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They should. at least right now. My thought was to always load this editor. And if it exists in that then we don't need to get the config element from the element its self.

Copy link
Copy Markdown
Member

@bramkragten bramkragten Sep 29, 2020

Choose a reason for hiding this comment

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

I don't like that it is not future proof, but also can't think of a case that will need it, so we'll see it when it happens I guess

@zsarnett
Copy link
Copy Markdown
Contributor Author

46b72046bd31fee6c024edaa5d263f35

This is something I am not sure on how to fix. Basically if an error occurs in the YAML then it breaks and goes to the code editor. I would rather it break here and require Yaml update for this row

entity: EntityId,
name: optional(string()),
icon: optional(Icon),
icon: optional(string()),
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.

why?

Copy link
Copy Markdown
Contributor Author

@zsarnett zsarnett Sep 28, 2020

Choose a reason for hiding this comment

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

Because idk why this was ever a thing :). If I am editing the icon. and I type anything but mdi:<Icon-name> then it is a struct error

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.

Oh, so it switches to the YAML editor?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same with the Entity ID. With this PR. I proposing using generic struct types and let the card error/warn about bad entity ids. Otherwise, trying to type an entity id into the code editor makes it jump because you haven't finished your input

@zsarnett
Copy link
Copy Markdown
Contributor Author

image

Comment on lines +37 to +41
if (!isValidEntityId((config as EntityConfig).entity!)) {
throw new Error(
`Invalid entity ID at position ${index}: ${config.entity}`
`Invalid entity ID at position ${index}: ${
(config as EntityConfig).entity
}`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not exactly sure what the best way to do this is. With the cast, it compains that entity doesn't exist in Type T.

Even if I add "entity" in config to the if statement.

@zsarnett
Copy link
Copy Markdown
Contributor Author

This PR Seems to be ever growing. Feel free to push to 0.117 as I have had to change the typings in a lot of places recently

@iantrich
Copy link
Copy Markdown
Member

iantrich commented Sep 30, 2020

Agree with 0.117. I can then wrap up changes to action editor/service_data using the same methodology and 0.117 can be a big editor release.

import { EntityConfig, LovelaceRowConfig } from "../entity-rows/types";

export const processConfigEntities = <T extends EntityConfig>(
export const processConfigEntities = <T extends LovelaceRowConfig>(
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.

This is not correct? We use this function for Glance etc? that is not extending LovelaceRowConfig? Should be EntityConfig?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should be both then

@internalProperty() private _guiModeAvailable? = true;

@query("hui-card-editor") private _cardEditorEl?: HuiCardEditor;
@query("hui-element-editor") private _cardEditorEl?: HuiElementEditor;
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.

We have hui-element so this is not a good name as we don't edit those?

zsarnett and others added 2 commits September 30, 2020 09:00
@bramkragten bramkragten merged commit a2ec878 into dev Sep 30, 2020
@bramkragten bramkragten deleted the entity-editor branch September 30, 2020 14:20
@bramkragten bramkragten mentioned this pull request Sep 30, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 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