Add weekday to time contidion editor

[Misiu]

Breaking change

Proposed change

Replacement of #6786 (I messed up rebase, it is a small change, so it was easier to create new PR)

This is the first step to add weekday's support to time condition.
Next will be ha-expand-panel (#6726) that will allow us to hide (collapse) parts that aren't filled

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New feature (thank you!)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example configuration

Additional information

• This PR fixes or closes issue: fixes Automations condition.weekday missing in Automation GUI #3833
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• There is no commented out code in this PR.
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

[Misiu]

@zsarnett could you take a look at this one?
It's a proper rebase of #6786 with all the comments and suggestions applied.

[zsarnett]

I'll let Bram take the final look before merging. Also, I have only reviewed the code and not the actual implementation and usability. I may look at that later today

[Misiu]

@zsarnett thank you for that.
I've checked the code before I created the initial PR, but core developer opinion is much appreciated.
Any chance this might get added to 0.115?

[zsarnett]

No unfortunately this will not be in 0.115. Beta Cut was yesterday. Bug fixes and follow up PRs only for this release

[Misiu]

@bramkragten could you take a look at this?
It was asked in WTH: https://community.home-assistant.io/t/wth-day-of-the-week-in-automations/226902

[bramkragten]

Why do we set the height to 40px? What is wrong with the default styling?

[Misiu]

It looks really bad without those 40px:

[bramkragten]

If you don't set any day, it will trigger at every day, right? That is not clear from the UI

[Misiu]

If you don't set any day, it will trigger at every day, right? That is not clear from the UI

Yes, that's right and I agree this isn't clear. Ideally, I should require at least one selected day and show warning/block saving if none are selected.
This PR is the first part. In the second part, I want to put the time part in one expansion panel and date in the second panel, so there will be much more space to add info.

What do you think about adding a title to the section and add a tooltip to it, something like here: #7006 (comment)

[bramkragten]

Why not just turn the logic around and enable all switches by default? That would reflect the actual behavior without the need for explaining?

[Misiu]

@bramkragten I can do that, but how to inform the user that he will get the same behavior if he selects all or none?

[bramkragten]

Saving it with none selected makes no sense, then it would not be true ever. So we should not allow that.

Btw, an empty array will not result in the same behavior as all selected. Only having the weekday not set at all will.
https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/condition.py#L526-L534

[Misiu]

Btw, an empty array will not result in the same behavior as all selected. Only having the weekday not set at all will.
https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/condition.py#L526-L534

If I understand that correctly if someone deselects all days then the condition will always be False? So right, that shouldn't be allowed.

Saving it with none selected makes no sense, then it would not be true ever. So we should not allow that.

Is there a way to prevent saving automation/mark condition as not valid? I can show an error message (custom logic) but maybe there is something I can reuse.

[bramkragten]

Well, we don't have to? As it would be logical that if you don't select any days it will not match ever?

[Misiu]

@bramkragten ok, I'll turn the logic around, and in future PR (with expansion panels) I'll add a tooltip with info that disabling all days will always make condition False.

[Misiu]

@bramkragten I've changed:

@property({ attribute: false }) public condition!: TimeCondition

to

  private _condition!: TimeCondition;

  set condition(value:TimeCondition) {
    this._condition = value;
    this._condition.weekday = this._condition.weekday || ["mon","tue", "wed","thu","fri", "sat","sun"];
  }

  get condition() {
    return this._condition;
  }

Is that fine? I want to assign all days if condition.weekday is empty or null.

[bramkragten]

Just set the switches to checked when !condition.weekday and when any is changed and !condition.weekday, let days = this.condition.weekday || Object.keys(DAYS) then do the same as before.

[Misiu]

@bramkragten please take a final look at this.
Now when there is no weekday in condition all days are displayed as selected. If someone doesn't change any day then the condition is saved without the weekday part (just like it used to do before), but if you deselect one day then the weekday part is generated and saved.

[Misiu]

@bramkragten @zsarnett thank you guys for the reviews!