Check config on save #2346
Check config on save #2346
Conversation
ghost
assigned 
src/panels/lovelace/hui-editor.ts
Outdated
| @@ -118,13 +124,30 @@ class LovelaceFullConfigEditor extends hassLocalizeLitMixin(LitElement) { | |||
|
|
|||
| private _handleSave() { | |||
| let value; | |||
| const text = this.textArea.value; | |||
|
|
|||
| if (text.includes("#")) { | |||
There was a problem hiding this comment.
Do we want to change this to # (with a space), so we don't catch colors?
There was a problem hiding this comment.
But # without space is also a comment, and we wouldn't warn... I think that's tricky...
There was a problem hiding this comment.
It's just a warning. I think it's fine to warn even if it's not truly a comment
There was a problem hiding this comment.
Imagine having an extra warning every time you click save because you have a color? That's annoying.
There was a problem hiding this comment.
True... idk... you're the boss 😉
There was a problem hiding this comment.
Can we set schema to FAILSAFE_SCHEMA ?
https://github.com/nodeca/js-yaml#safeload-string---options-
There was a problem hiding this comment.
There was a problem hiding this comment.
Okay, so this is difficult. What about we show a warning below the editor if we notice any # . ?
There was a problem hiding this comment.
I think that meets every concern in the middle and has my vote
There was a problem hiding this comment.
Why not first get rid of all # within hex color codes using a regex #([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3}) then throw a warning or error for any other #?
Untested...
Could miss some comments...
Warn on # at the start of a line or when followed by white space
bramkragten
force-pushed
the
check-config-save
branch
from
c1d56b3
to
774f397
Compare
|
|
||
| if (this._hashAdded) { | ||
| if ( | ||
| !confirm( |
There was a problem hiding this comment.
I don't think that we need to do this confirm because we're showing a warning?
| } | ||
|
|
||
| private _yamlChanged() { | ||
| this._hash = this._hashAdded || this.textArea.value.includes("#"); |
There was a problem hiding this comment.
I think that we should skip this. I think that it's perfect to only check if a user adds a hash. That way we don't have to worry about any colors that already exist.
There was a problem hiding this comment.
The only problem is when a user pastes something but might be an edge case (it is possible to check the pasted content but might be overkill...)
| ></paper-icon-button> | ||
| <div main-title>Edit Config</div> | ||
| ${ | ||
| this._hash |
There was a problem hiding this comment.
Feels a bit weird to put in the title. If there is not enough space (ie on mobile), it will look very weird. It also won't work as an element above the text area because showing it will push the whole textarea down, and that's annoying. Wonder if this should be a toast that's shown when entering a # character?
There was a problem hiding this comment.
I was thinking about a toast too but figured it should be close to the save button...
Warns when
#is found, but can also be a color or anything with a#in a string...