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

Automations condition.weekday missing in Automation GUI #3833

Closed
ghost opened this issue Sep 29, 2019 · 10 comments · Fixed by #6848
Closed

Automations condition.weekday missing in Automation GUI #3833

ghost opened this issue Sep 29, 2019 · 10 comments · Fixed by #6848
Labels

Comments

@ghost
Copy link

ghost commented Sep 29, 2019

The weekday interface in the Automations UI is missing.
I have the following config:

- id: '1555796295451'
  alias: Verwarming uit MA DI WO DO
  trigger:
  - at: '11:30'
    platform: time
  condition:   
  - condition: time
    weekday:
    - mon
    - tue
    - wed
    - thu
  action:
  - data:
      entity_id: climate.woonkamer,climate.serre
      temperature: 16
    service: climate.set_temperature
  - data:
      entity_id: automation.schuifpui_dicht_zet_verwarming_aan
    service: automation.turn_off

But the interface looks like this
image
It would be nice to have a or something to select the weekday time condition and then a list of checkboxes/switches to add or remove the weekdays that will be added in the condition.

@frenck frenck transferred this issue from home-assistant/core Sep 30, 2019
@iantrich iantrich added the help-wanted In need of Additional Help label Oct 4, 2019
@Misiu
Copy link
Contributor

Misiu commented Mar 9, 2020

Currently, I have something like this (WIP):
image

TimeCondition interface has two fields: after and before, I've added the third field that holds weekdays, but while working on this I get a feeling that we shouldn't mix time and weekday in a single condition.
I think that weekday should be added as a separate condition type:
image

I'm aware that weekday is part of the time condition (https://www.home-assistant.io/docs/scripts/conditions/#time-condition), but you can add two conditions for time and for weekday with AND condition.

I propose to split that condition into two:
image

@bramkragten @iantrich do you think?
Not sure what changes need to be added to the front and to the backend, but before I do anything I'd like to discuss this.

The easiest thing to do would be to replace time condition (that contains weekday) with two conditions inside AND condition.

So instead of:
image

you would get:
image

@bramkragten
Copy link
Member

I think the split is a good idea UX wise, but I think it is good idea to keep the frontend and backend/yaml aligned and that would be breaking... Can you create an architecture issue for this?

@Misiu
Copy link
Contributor

Misiu commented Mar 9, 2020

@bramkragten created: home-assistant/architecture#356
Could you please add an upvote for it? :)

@Misiu
Copy link
Contributor

Misiu commented Mar 23, 2020

@bramkragten I've added #5324.
It's my first PR that requires frontend, backend, docs and architecture changes.
I'm not sure about breaking changes policy, but for now, I left weekday property in time condition, this will be removed in 0.109, or future release if change requested.

@Misiu
Copy link
Contributor

Misiu commented Apr 20, 2020

@bramkragten what do you think about this?
I got some feedback on home-assistant/architecture#356 by @MartinHjelmare and I'm not sure if this will get approved.
As I wrote before I can add weekdays to current condition editor, but I think it should be split into smaller editors (time and weekday condition).
Can I ask for your opinion?

@zsarnett
Copy link
Contributor

@Misiu Can these be faked? As in, the UI shows the conditions as weekday or time but really the yaml is written how it is now?

@bramkragten
Copy link
Member

@Misiu Martin is right there, we can already do what you want without a backend change.

@Misiu
Copy link
Contributor

Misiu commented Jul 20, 2020

I'd like to get back to this.
I can put the weekday condition part in accordion, for example, vaadin-accordion.
This will look like this (don't look at style):
image

But as mentioned before my main motivation was to split weekday and time into two separate conditions.
If you have automation that needs to run three times a day on specific days you can add one condition for days and a second (a group with or) for hours. This will take less space than 3 large conditions showing time and days.
There is one bonus: If you want to disable automation for a specific day you will only edit it in one place.

@zsarnett as @bramkragten wrote here we should align frontend and backend.
I'm not a UX designer and I don't have idea what would be the best design for this editor.

WDYT about the accordion approach? By default, it would be collapsed, but when at least one day is selected I can expand it.

@Misiu
Copy link
Contributor

Misiu commented Aug 27, 2020

@bramkragten another prototype:https://stackblitz.com/edit/ha-expand-panel?file=index.html

7b8bfa0da4e36eef2c4f2583d6a86403

I've created ha-expand-panel component.
Idea is to expand Weekday's condition when there is at least one day selected.
Not sure how to handle hours, but if the inputs will be empty then I can collapse that section too.

I'm using mwc-formfield because this allows clicking on label to change switch value.
I'd also would like to use ha-textfield component instead of paper-input because we can set type (time in this case).

@emontnemery
Copy link
Collaborator

@Misiu I think this looks nice!

@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants