Convert hui-markdown-card to TypeScript/LitElement #1808
Conversation
ghost
added
the
|
|
||
| public setConfig(config: Config) { | ||
| if (!config.content) { | ||
| throw new Error("content required"); |
There was a problem hiding this comment.
Invalid Config: Content Required
| return html` | ||
| ${this.renderStyle()} | ||
| <ha-card .header="${this.config.title}"> | ||
| <ha-markdown content="${this.config.content}"></ha-markdown> |
There was a problem hiding this comment.
Pretty sure you can use .content here
| -webkit-user-select: initial; | ||
| -moz-user-select: initial; | ||
| } | ||
| :host([no-title]) ha-markdown { |
There was a problem hiding this comment.
You removed this CSS and logic ???
There was a problem hiding this comment.
padding-top: ${this.config!.title ? '0' : '16px'};
I believe this line covers all the previous CSS and logic that was removed
| title?: string; | ||
| } | ||
|
|
||
| export class HuiMarkdownCard extends HassLocalizeLitMixin(LitElement) |
There was a problem hiding this comment.
Why include HassLocalizeLitMixin?
There was a problem hiding this comment.
Should I just extend LitElement instead?
There was a problem hiding this comment.
HassLocalizeLitMixin provides a localize function for translating text. Since you're not using that, export class HuiMarkdownCard extends LitElement should be enough, yes.
There was a problem hiding this comment.
Okay. I should probably create a new pull to clean up my other conversion as well then: #1801
Thanks
| ha-markdown { | ||
| display: block; | ||
| padding: 0 16px 16px; | ||
| padding-top: ${this.config!.title ? '0' : '16px'}; |
There was a problem hiding this comment.
We shouldn't make stylesheets dynamic, as it means the whole CSS stylesheet has to be re-rendered. I suggest you use a styleMap (example)
There was a problem hiding this comment.
Note that in that case, you could also use a classMap (example), which I think I prefer, as that means we keep all styling inside renderStyle, meaning it can be overridden easily.
There was a problem hiding this comment.
Great! Thanks! I'll put in a PR for this as well:
| return html` | ||
| ${this.renderStyle()} | ||
| <ha-card .header="${this.config.title}"> | ||
| <ha-markdown class="markdown ${classMap({ |
There was a problem hiding this comment.
Lets put each attribute/property on its own line example
There was a problem hiding this comment.
This is only if there is multiple. So ha-card with .header only is okay
|
Seems ready after small change |
No description provided.