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

Hide wind speed and direction #190

Merged
merged 10 commits into from
Nov 3, 2022
Merged

Hide wind speed and direction #190

merged 10 commits into from
Nov 3, 2022

Conversation

akropp
Copy link
Contributor

@akropp akropp commented Oct 20, 2022

Wanted to be able to only show the wind speed (without the direction). For completeness, added two flags: hide_wind_speed and hide_wind_direction. They work in concert with show_wind, and default to false (so the existing behavior).

Copy link
Owner

@decompil3d decompil3d left a comment

Choose a reason for hiding this comment

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

Thanks so much for your contribution! I like the idea of being able to show only wind speed or direction. I think that we can achieve the same result without adding more flags though.

Can you please refactor to make show_wind take these values:

  • true: show both wind speed and direction
  • false: show neither speed nor direction
  • 'speed': show only wind speed
  • 'direction': show only wind direction

Then, in the editor UI, you can just replace the existing wind switch with a drop-down or similar to choose from these options.

What do you think?

@akropp
Copy link
Contributor Author

akropp commented Oct 20, 2022 via email

@akropp
Copy link
Contributor Author

akropp commented Oct 24, 2022 via email

@decompil3d
Copy link
Owner

Thanks for updating. I'll probably have time to look more next week. In the meantime, I'll toss on the hacktoberfest-accepted label in case you're participating in that.

Copy link
Owner

@decompil3d decompil3d left a comment

Choose a reason for hiding this comment

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

Played with it a little and I do see the editor behavior you described. However, that same behavior happens on the entity field and that logic comes from the base custom card template repo. So, I'm inclined to just let it be as-is. If someone really wants to manage their cards in YAML and needs to mess with this field, then they can either use the dashboard-level YAML editor, or they can compose elsewhere and paste in to avoid the partial text validation problem.

Just a small refactor request for how you render the wind elements and I think we'll be good to go.

src/weather-bar.ts Show resolved Hide resolved
src/weather-bar.ts Outdated Show resolved Hide resolved
akropp and others added 3 commits November 3, 2022 10:56
Co-authored-by: Jonathan Keslin <decompil3d@users.noreply.github.com>
Co-authored-by: Jonathan Keslin <decompil3d@users.noreply.github.com>
@akropp
Copy link
Contributor Author

akropp commented Nov 3, 2022

Sounds good.

@decompil3d decompil3d merged commit 3f7aa78 into decompil3d:main Nov 3, 2022
@akropp akropp deleted the wind_speed_and_direction branch November 7, 2022 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants