Adds an option to fill the forecast segment with icons #715
Conversation
| | `hide_temperatures` | bool | **Optional** | Whether to hide temperatures under the bar | `false` | | ||
| | `round_temperatures` | bool | **Optional** | Whether to round temperatures to the nearest whole number | `false` | | ||
| | `hide_bar` | bool | **Optional** | Whether to hide the bar itself | `false` | | ||
| | `icon_fill` | [string][icon_fill] | **Optional** | Whether to repeat the icon inside the bar | `'single` | |
There was a problem hiding this comment.
This the only real change in this file. Everything else is adding spaces to keep columns aligned.
src/hourly-weather.ts
Outdated
| @@ -196,6 +196,11 @@ export class HourlyWeatherCard extends LitElement { | |||
| } | |||
| } | |||
|
|
|||
| if (config.icon_fill && config.icon_fill !== 'single' && config.icon_fill !== 'full') { | |||
| if (!Number.isNaN(config.icon_fill) && (config.icon_fill < 1)) { | |||
| throw new Error(this.localize('errors.must_be_positive_int')); | |||
There was a problem hiding this comment.
Let's not reuse this error string, as it won't be very helpful to users if they mistype the value. Can you make a new string in src/localize/languages/en.json under the errors section with something like this:
icon_fill must be either a positive integer or one of "single" or "full"
No need to translate to other languages. Once I merge this, I can trigger auto-translation on the new string before releasing.
There was a problem hiding this comment.
Got it. I also refactored the check to be more explicit about what is a valid value and what isn't.
|
@decompil3d I think I need some help with checking the config option
Why do we need two? Which one should I use to test the validity of |
I think checking validity in |
|
@decompil3d added tests, this PR is ready for review now |
…for #715) (#726) * Adds an option to fill the forecast segment with icons (#715) * Adds an option to fill the forecast segment with icons * Fix typos * Improved error detection and error message * undefined icon_fill behaves like icon_fill:single * Move icon_fill config check to renderCore * Tests for icon_fill errors * Add tests for new behaviour * [Auto] Adding updated localization files Files changed: M src/localize/languages/cs.json M src/localize/languages/da.json M src/localize/languages/de.json M src/localize/languages/es.json M src/localize/languages/fr.json M src/localize/languages/hu.json M src/localize/languages/it.json M src/localize/languages/nb.json M src/localize/languages/nl.json M src/localize/languages/nn-NO.json M src/localize/languages/pl.json M src/localize/languages/pt-BR.json M src/localize/languages/pt.json M src/localize/languages/ru.json M src/localize/languages/sk.json M src/localize/languages/zh.json * Massage the auto-translation a bit * [Auto] Adding updated localization files Files changed: M src/localize/languages/pt-BR.json * Add note to README about hidden icons on narrow segments * Update readme table * Tiny edit on readme --------- Co-authored-by: Sergio Cinos <[email protected]>
(Comes from #669)
What problem does this PR solve?
Adds the option to add more than one icon per forecast span. This should improve readability, specially for wallboards.
Proposed solution
Adds a new config
icon_fillwith three possible values:singleThis is the default option and keeps current functionallity: it shows one single icon per span, centered.fullShow one icon per forecast segment. For example, it will display 12 sun icons if the next 12h is sunny.<n>(an integer). Likefull, but instead of displaying an icon per forecast segment, it will displaye an icon every nth segment. (In other words,fulland1do the same). At the very least it will display one icon per span.This is how it looks like:
Alternatively, we can have this as a boolean (
icon_fill:falsecurrent behaviour,icon_fill:truefill all segments with icons), and introduce a second optionicon_spacing(similar tolabel_spacing) to be able to space out the icons