AalianKhan / mushroom-strategy

A strategy to automatically generate a dashboard using mushroom cards
MIT License
373 stars 37 forks source link

Added fix for weather entity error #6

Closed AalianKhan closed 1 year ago

AalianKhan commented 1 year ago

Add option for weather entity to be configured If the weather entity is not configured, then it searches for the weather entity and pushes a weather chip if exists

DigiLive commented 1 year ago

I think all does what is expected.

To be honest, when I saw your PR, I hoped for a review request before you would merge the branch into main and delete it. Meant as constructive criticism, I probably would have coded the weather-chip something like this:

// Weather Chip
const weatherEntityId = strategyOptions.chips?.weather_entity ??
  entities.find(
    entity => entity.entity_id.startsWith("weather.") && entity.disabled_by == null && entity.hidden_by == null
  )?.entity_id;

if (weatherEntityId) {
  chips.push({
      type: "weather",
      entity: weatherEntityId,
      show_temperature: true,
      show_conditions: true
  });
}

I've also noticed the formatting of your JavaScript doesn't come close to mine. If you're interested, I can reformat the code for you a bit for better readability. From there, maybe we can cleanup and optimize the code.

AalianKhan commented 1 year ago

This is my very first JavaScript so that's probably the reason 😅 (just searched up what that question mark does)

Yeah I would definitely love to make the code more readable and see my mistakes.

Also figuring out GitHub aswell

DigiLive commented 1 year ago

This is my very first JavaScript so that's probably the reason 😅 (just searched up what that question mark does)

Yeah I would definitely love to make the code more readable and see my mistakes.

If the code behaves as expected, there are no mistakes. 😉 Many ways of coding, lead to the same results. Experience leads to a more readable or optimized result. Actually my development core is PHP. All I know about javascript is by years of experience and trail and error.

Also figuring out GitHub aswell

You'll learn by experience. Now and then, I have to refer to the manual also.

Maybe, but it's totally up to you, you can add me to the repo as a collaborator so I can guide you a little bit. Otherwise you can contact me by email.

Once you agree, I'll clone the repo and create a new PR to get started. It would help if you don't change the code in the meanwhile to prevent merge conflicts.

AalianKhan commented 1 year ago

Yeah totally, I will add you as a collaborator. I thought anyone could contribute to repositories. Thanks a lot

DigiLive commented 1 year ago

Yes. Anyone can contribute to a public repo. A collaborator can do a bit more.

Take a look at https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-personal-account-settings/permission-levels-for-a-personal-account-repository for more details.

If you have any questions or remarks about my activities, contact me by email. If you do, I'll send you some guidelines for commiting changes to keep you history more clear.

AalianKhan commented 1 year ago

Hi again! So I was thinking about some ideas for the future of this project and strategies as a whole. I think these strategies should be more basic I think that there should be base strategies that basically tell the basic layout of the views and have customization options Some examples of a base strategy can be a room subview-based layout or a 1-page room layout etc... By default, these base strategies use basic built-in cards already present in Home Assistant.
If users want to define their own cards for a platform, device classes or specific entities they can via options. What do you think about this? I was trying to look for your email but couldn't find it. mine is posted in my GitHub profile.