-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: improve state card performance #1
Conversation
src/cards/state.ts
Outdated
export function handleState(context) { | ||
if (context.cardType !== 'state') { | ||
createStructure(context) | ||
} | ||
`; | ||
|
||
addStyles(hass, context, buttonStyles, customStyles, state, entityId, stateChanged); | ||
} No newline at end of file | ||
updateValues(context); | ||
} |
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.
The handle states could be called i two situations:
- The card creation, where we will initialize the structure with
createStructure
- The hass object update, where we will just update what is needed with
updateValues
.
addStyles
is removed because the style is either in the constant below or updated in updateValues
src/cards/state.ts
Outdated
function getIcon(context) { | ||
const entityIcon = context._hass.states[context.config.entity]?.attributes.icon; | ||
const configIcon = context.config.icon; | ||
|
||
if (configIcon) return configIcon; | ||
if (entityIcon) return entityIcon; | ||
|
||
return ''; | ||
} |
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.
I have extracted functions from getVariables
so I can call only the ones I need.
This method would probably need to be update on the other type of card and at this time, moved to another file.
It's the same for getImage
, getName
and getState
src/cards/state.ts
Outdated
function createStateCardContainer() { | ||
const stateCardContainer = document.createElement("div"); | ||
stateCardContainer.classList.add("button-container"); | ||
|
||
return stateCardContainer; | ||
} |
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.
All these create methods might bea bit overkilled and could be brought back in the createStructure
method.
It was just clearer to me when tryiing to split some part of the code.
src/cards/state.ts
Outdated
context.elements = {}; | ||
context.elements.stateCardContainer = createStateCardContainer(); | ||
context.elements.stateCard = createStateCard(context); | ||
context.elements.nameContainer = createNameContainer(); | ||
context.elements.iconContainer = createIconContainer(); | ||
context.elements.name = createName(); | ||
context.elements.state = createState(); | ||
context.elements.feedback = createFeedback(); | ||
context.elements.icon = createIcon(); | ||
context.elements.image = createImage(); | ||
context.elements.style = createStyle(); |
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.
I put every dom created into the context.elements object so we can update everything from the updateValues
method
context.content.appendChild(context.elements.stateCardContainer); | ||
context.content.appendChild(context.elements.style); | ||
|
||
context.cardType = 'state'; |
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.
This will let the code know that the card has already been initialized.
if (name !== context.elements.name.innerText) { | ||
context.elements.name.innerText = name; | ||
} |
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.
This is the only place where I check for the difference: changing a style will not trigger any change in the DOM if the value is the same but innerText
or innerHTML
will
const formattedState = context._hass.formatEntityState(context._hass.states[context.config.entity]); | ||
|
||
if (state === 'unavailable') { | ||
context.card.style.opacity = '0.5'; |
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.
Rather than having it in the style append, it's manually updated inline here so the execution time would be better.
context.elements.iconContainer.appendChild(context.elements.icon); | ||
context.elements.iconContainer.appendChild(context.elements.image); |
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.
I place both image and icon in the DOM,but since one of them would be hidden, it's not a big deal on performance. It will avoid the need to handle the creation/removal of the dom nodes based on the state.
It's also the case of the feedback element and the state
} | ||
|
||
const stateCardStyles = ` |
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.
Style is now only static string. This way, it's only included once when the component is mounted
I've just looked at your code and thanks a lot for that! I still consider myself as a beginner, Bubble Card is my first (big) training project and I'm learning everything by myself. So thanks again for that! I can't wait to try and learn from it! |
09067f1
to
a3e5ac7
Compare
7d30bab
to
bd6101b
Compare
bd6101b
to
732b2fd
Compare
No description provided.