fix: update alarm tile example with support for new states #472
Conversation
|
@akloeckner Care to review? |
| @@ -8,6 +8,7 @@ extends: 'eslint:recommended' | |||
| ignorePatterns: | |||
| - build/ | |||
| - scripts/vendors/ | |||
| - config.example.js | |||
There was a problem hiding this comment.
Why should we not lint the example? Probably that's the piece of code that is most read by the user...
There was a problem hiding this comment.
We could lint but it would then need to use eslint configuration that doesn't use any new ES features as the config needs to, unfortunately, stay compatible with old browsers and we can't transpile it.
There was a problem hiding this comment.
Ah, I see the dilemma. Let's just run eslint --fix from time to time then, to have at least indents and the like correct.
(In the long run, I'd prefer to have only one file with examples. That should render on GitHub and be usable as a start configuration. Maybe like here: https://github.com/akloeckner/TileBoard/blob/test/one-example/TILE_EXAMPLES.md . But if you have better ideas...)
There was a problem hiding this comment.
eslint --fix will still use the default configuration so it will switch to new ES syntax if we run it.
We can just have an override for this file specifically and un-ignore it then.
I agree about having one place with examples.
| @@ -178,26 +178,32 @@ var CONFIG = { | |||
| { | |||
| position: [0, 1], | |||
| type: TYPES.ALARM, | |||
| //id: "alarm_control_panel.home_alarm", | |||
| // id: "alarm_control_panel.home_alarm", | |||
| id: { state: 'disarmed' }, // replace it with real string id | |||
There was a problem hiding this comment.
I'd say, the effective setting in the example should be the actual example id, while this line here appears to be a test. (Which I appreciated very much in testing...)
There was a problem hiding this comment.
I suppose it's done like that because that will make the tile show up while real ID wouldn't (it would complain about the missing entity). So I think it's better like that for the purpose of showing something to user.
There was a problem hiding this comment.
OK. Sounds reasonable. We should apply that thinking to the other examples then, too. In the long run...
| disarmed: 'Disarmed', | ||
| pending: 'Pending', | ||
| armed_custom_bypass: 'Armed custom bypass', |
There was a problem hiding this comment.
This state name is so long, it overflows my tile. While that is probably another issue by itself, I suggest to use a shorter name in the example. Maybe just “Custom“?
There was a problem hiding this comment.
I think I'll go with "Armed bypass". I want to preserve the text that alarm is armed to be consistent with other armed states.
No description provided.