-
Notifications
You must be signed in to change notification settings - Fork 233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add show_legend_state #1082
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -338,17 +338,29 @@ class MiniGraphCard extends LitElement { | |
|
||
renderLegend() { | ||
if (this.visibleLegends.length <= 1 || !this.config.show.legend) return; | ||
|
||
|
||
return html` | ||
<div class="graph__legend"> | ||
${this.visibleLegends.map(entity => html` | ||
<div class="graph__legend__item" | ||
@click=${e => this.handlePopup(e, this.entity[entity.index])} | ||
@mouseenter=${() => this.setTooltip(entity.index, -1, this.getEntityState(entity.index), 'Current')} | ||
@mouseleave=${() => (this.tooltip = {})}> | ||
${this.renderIndicator(this.entity[entity.index].state, entity.index)} | ||
<span class="ellipsis">${this.computeName(entity.index)}</span> | ||
</div> | ||
`)} | ||
${this.visibleLegends.map((entity) => { | ||
let legend = this.computeName(entity.index); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please properly indent the code. |
||
const state = this.getEntityState(entity.index); | ||
|
||
const { show_legend_state } = this.config.entities[entity.index]; | ||
if (show_legend_state) { | ||
legend += ` (${this.computeState(state)}${this.computeUom(entity.index)})`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you put the state in an additional I think there is a CSS property then to add the braces to the content. That way users could use Please make sure the defaults look good in different situations, e.g. long and short state names. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Absolutely correct. Also, please think about an order: in some cases a user may want to have UoM before a value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think, a separate |
||
} | ||
|
||
return html` | ||
<div class="graph__legend__item" | ||
@click=${e => this.handlePopup(e, this.entity[entity.index])} | ||
@mouseenter=${() => this.setTooltip(entity.index, -1, state, 'Current')} | ||
@mouseleave=${() => (this.tooltip = {})}> | ||
${this.renderIndicator(this.entity[entity.index].state, entity.index)} | ||
<span class="ellipsis">${legend}</span> | ||
</div> | ||
`; | ||
})} | ||
</div> | ||
`; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though this is not the case for most of the other entries: please state the default value as false. Let's be a good example. ;-)