Skip to content
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

Add Magic areas 2 #75

Closed

Conversation

adrienbrault
Copy link
Contributor

Hey,

This PR contains #44 rebased, along with some refactoring based on @DigiLive's comment.

Here's what it looks like on my dashboard:

@DigiLive DigiLive mentioned this pull request Oct 24, 2023
@adrienbrault adrienbrault force-pushed the magic-areas-refactored branch from de49339 to 7d203c5 Compare October 24, 2023 08:12
@adrienbrault
Copy link
Contributor Author

@DigiLive I've updated the branch with your suggestions.

I think the configuration format should be improved though

@DigiLive
Copy link
Collaborator

I think the configuration format should be improved though

Which format of which configuration do you suggest to what? Do you have a code example?

@@ -355,7 +356,15 @@ class Helper {

// Get states whose entity-id starts with the given string.
const stateEntities = Object.values(this.#hassStates).filter(
state => state.entity_id.startsWith(`${domain}.`),
state => {
if (deviceClass
Copy link
Collaborator

@DigiLive DigiLive Oct 24, 2023

Choose a reason for hiding this comment

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

Can the return conditions rewritten to a ternary operator statement?
E.g.: return deviceClass && state.attributes.device_class === deviceClass ? state.entity_id.startsWith(`${domain}.`) : false;

*
* @return {stateObject[]} Array of state entities.
*/
static getStateEntities(area, domain) {
static getStateEntities(area, domain, deviceClass = null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

deviceClass = null or deviceClass = undefined?

state => state.entity_id.startsWith(`${domain}.`),
state => {
if (deviceClass
&& state.attributes.device_class !== deviceClass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unresolved variable device_class.

It should be included in typedef stateObject in file src/typedefs.js


if (!card.secondary) {
// Get or determine the relevant sensor entity IDs based on options or default behavior
const temperatureSensorId = this.options?.temperature || Helper.getStateEntities(this.entity, "sensor", "temperature")[0]?.entity_id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The type of field options in AbstractClass should change to {abstractOptions & Object<string, any>} to allow unknown additional properties.

Applies to lines #L70, #L71, #L72, #L92, #L93, #L94.


if (!card.secondary) {
// Get or determine the relevant sensor entity IDs based on options or default behavior
const temperatureSensorId = this.options?.temperature || Helper.getStateEntities(this.entity, "sensor", "temperature")[0]?.entity_id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Argument type  hassEntity | areaEntity is not assignable to parameter type areaEntity.

Maybe it's more proper to declare private field #entity in AreaCard as type areaEntity and assign in the value of parameter area in the constructor. And then use this.#entity in the function call of Helper.getStateEntities

Applies to lines #L70, #L71, #L72, #L92, #L93, #L94.

*
* The result excludes hidden and disabled entities.
*
* @param {areaEntity} area Area entity.
* @param {string} domain Domain of the entity-id.
* @param {string|null} deviceClass Device class attribute of the state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

deviceClass should be [deviceClass] if it's an optional parameter.

@DigiLive
Copy link
Collaborator

Please remove ./dist/mushroom-strategy.js from your commits.
It will create a merge conflict. The file will be build at GitHub when an applicable PR is merged into the main branch.

@DigiLive DigiLive linked an issue Oct 24, 2023 that may be closed by this pull request
@DigiLive DigiLive added the enhancement New feature or request label Nov 22, 2023
@DigiLive
Copy link
Collaborator

Warning

The main branch has been switched to TypeScript.
Update your feature branch accordingly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Room button badges/actions
3 participants