frontend icon indicating copy to clipboard operation
frontend copied to clipboard

Handle sequences under parallel actions

Open surfingbytes opened this issue 1 year ago • 29 comments

Proposed change

Currently, parallel action can run only single actions in parallel. However, you may need to run sequences of actions in parallel, for example sequence of actions "Robo vac go to bathroom", "wait for mop to be replaced", "clean house" in parallel with another sequence "morning announcements". Currently, when parallel action contains sequence, it is not possible to edit in UI. This change allows to display it. Most logic was copied from "choose action"- without conditions of course. Right before the parallel action is rendered, it converts all standalone actions into sequence, so everything is displayed the same way.

This is replacement of stale PR 14166 which is impossible to update to latest codebase.

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] 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 #18725
  • This PR is related to issue or discussion: https://github.com/home-assistant/frontend/pull/14166
  • Link to documentation pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [x] 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:

surfingbytes avatar Oct 30 '23 09:10 surfingbytes

I think it would be helpful for review purposes if you include a screenshot of what your proposed UI looks like.

Personally I do feel like this is an important missing component in the UI editor, I was actually looking for something like this recently and didn't even realize that parallel actions supported this sequential mode. However IMO I'm not sure that making every action into a sequence is the best UI, feels a bit cluttery to me, and possibly a bit confusing, as there are now multiple "add action" buttons on the screen, and it may not be explicitly clear that these groups are executed sequentially, I didn't see any indication of "sequential" in the displayed text.

Maybe one possibility could be that when you click on "add action" in a parallel block, one of the choices in the dropdown could say "Run in sequence", and then selecting that would add the sequential action block, and leave the other items displayed as usual? Just a thought.

karwosts avatar Nov 07 '23 17:11 karwosts

I'd like to add screenshots, but my docker updated and now it's not possible to run HA in dev container - see https://community.home-assistant.io/t/failing-to-build-a-dev-container/529524/2 However, it's looks very much like "Choose" action..

surfingbytes avatar Nov 15 '23 10:11 surfingbytes

@karwosts , I managed to make my prod home assistant to use development frontend, and gt the screenshot from it. As you see, it's very similar to "choose" action. The main button, adding new "parallel branch", is called "Add parallel actions", while inside branch, the button is "Add action", same as everywhere else: image

surfingbytes avatar Nov 27 '23 08:11 surfingbytes

@matthiasdebaat , is there anything else needed to merge this?

surfingbytes avatar Dec 17 '23 14:12 surfingbytes

I'd love to see this added!

Californian avatar Dec 27 '23 23:12 Californian

I would also love to see this added.

jmpeter avatar Jan 06 '24 15:01 jmpeter

@matthiasdebaat , is there anything else needed to merge this?

Sorry, I was a couple of weeks offline. Thanks for making a UI for this! To fully understand it, can you share the YAML of your example?

In conditions, every option is also collapsible. Is there a reason why you didn't use that same pattern here?

CleanShot 2024-01-08 at 11 40 54@2x

matthiasdebaat avatar Jan 08 '24 10:01 matthiasdebaat

Good idea about makingit collapsible, will look into it.

parallel:
  - sequence:
      - wait_for_trigger:
          - platform: state
            entity_id:
              - switch.hall_switch_main_door_l1
            from: "off"
            to: "on"
            for:
              hours: 0
              minutes: 0
              seconds: 0
          - platform: state
            entity_id:
              - binary_sensor.hall_entrance_door_contact
            from: "on"
            to: "off"
            for:
              hours: 0
              minutes: 0
              seconds: 0
      - if:
          - condition: state
            entity_id: switch.hall_switch_main_door_l1
            state: "on"
        then:
          - service: switch.turn_off
            data: {}
            target:
              entity_id: switch.hall_switch_main_door_l1
          - service: script.stop_automation
            data:
              automation: automation.hall_apartment_enter_leave
  - sequence:
      - delay: "00:00:01"
      - service: pyscript.check_device
        data:
          entity_id: device_tracker.tomas_phone_ip
          ip: ****
  - sequence:
      - delay: "00:00:01"
      - service: pyscript.check_device
        data:
          entity_id: device_tracker.zuzik_phone_ip
          ip: ****
  - sequence:
      - wait_for_trigger:
          - platform: state
            entity_id:
              - device_tracker.tomas_phone_ip
        timeout: "00:05:00"
        continue_on_timeout: false
      - if:
          - condition: state
            entity_id: group.apartment_members
            state: home
        then:
          - delay:
              hours: 0
              minutes: 0
              seconds: 10
              milliseconds: 0
          - service: automation.trigger
            data:
              skip_condition: true
            target:
              entity_id: automation.bathroom_washing_machine_notifications
        else:
          - if:
              - condition: state
                entity_id: binary_sensor.dishwasher_common_status_doorstate
                state: "off"
              - condition: state
                entity_id: sensor.dishwasher_operation_state
                state: BSH.Common.EnumType.OperationState.Inactive
            then:
              - service: button.press
                data: {}
                target:
                  entity_id: button.dishwasher_start_pause
                continue_on_error: true
          - delay:
              hours: 0
              minutes: 15
              seconds: 0
              milliseconds: 0
          - if:
              - condition: template
                value_template: >-
                  {{ state_attr('script.roborock_clean_apartment_once_a_day',
                  'last_triggered').date() <= now().date() - timedelta(days=1)
                  }}
            then:
              - service: script.turn_on_for_time
                data:
                  switch: switch.water_pump_strelitzia
                  time:
                    hours: 0
                    minutes: 0
                    seconds: 10
              - service: script.roborock_clean_apartment_once_a_day
                data: {}
    enabled: true

surfingbytes avatar Jan 08 '24 11:01 surfingbytes

Not sure why it messes up the formatting when I put it in as "code" though...

surfingbytes avatar Jan 08 '24 11:01 surfingbytes

If you create it with a collapsible like conditions, I think it's good UI-wise.

matthiasdebaat avatar Jan 08 '24 13:01 matthiasdebaat

It's now collapsible and supports other actions (re-order, rename..) same as "Choose" action. It also hs better description so now it's clear the sequences are run in parallel to each other, not the actions inside them:

image

surfingbytes avatar Jan 17 '24 09:01 surfingbytes

Is there anything else needed @bramkragten @matthiasdebaat ?

surfingbytes avatar Jan 23 '24 09:01 surfingbytes

UI looks great, thanks!

matthiasdebaat avatar Jan 23 '24 10:01 matthiasdebaat

Cool, can it be merged then, please?

surfingbytes avatar Jan 24 '24 09:01 surfingbytes

Yes please, i am very much looking forward to this feature, since i am currently migrating my automations from AppDaemon to HA native, and i would need this UI element to complete the last automation which i would need to create in YAML otherwise.

simonszu avatar Jan 24 '24 09:01 simonszu

I updated the PR to work with the latest updates of the automation reorder modes.

About the PR itself:

I think it is OK to add sequences to parallel actions, but I don't think this should be the default.

Parallel actions are already pretty complicated and adding another layer in the form of sequences makes it even more complicated.

I would propose to keep the default as single actions, and add a new button Add sequence next to the Add action button.

Sequences will then be supported in the UI, but the current experience is not made more complicated.

bramkragten avatar Jan 30 '24 23:01 bramkragten

Sequence actions also added via UI is very welcome, i handle mine via yaml in nested parallels but having them clean via the new building block would be very nice.

r0bb10 avatar Feb 24 '24 09:02 r0bb10

I updated the PR to work with the latest updates of the automation reorder modes.

About the PR itself:

I think it is OK to add sequences to parallel actions, but I don't think this should be the default.

Parallel actions are already pretty complicated and adding another layer in the form of sequences makes it even more complicated.

I would propose to keep the default as single actions, and add a new button Add sequence next to the Add action button.

Sequences will then be supported in the UI, but the current experience is not made more complicated.

Yes, that seems like a good solution. @surfingbytes is there any chance you are willing to implement the proposed solution?

tieskuh avatar Mar 13 '24 16:03 tieskuh

I would propose to keep the default as single actions, and add a new button Add sequence next to the Add action button.

Sequences will then be supported in the UI, but the current experience is not made more complicated.

That is not possible to do in ha-automation-action-parallel. However, it will be possble to do by creating new class ha-automation-action-sequence, which could then be added anywhere via either "Add action" or "Add building block" -> there would be new action type "sequence". To me, it looks like a clean solution, but @bramkragten please confirm, before I start working on it. Thank you.

surfingbytes avatar Mar 25 '24 14:03 surfingbytes

Just to warn a sequence is not a valid building block, so allowing it to be added to other places where building blocks can be used will generate invalid automations.

karwosts avatar Mar 25 '24 15:03 karwosts

Thank you. I'll create this new panel type for sequence, and instead of adding it to standard action types, I'll just add new button to parallel action.

surfingbytes avatar Mar 25 '24 15:03 surfingbytes

Can you please check it again @karwosts ? Parallel component is reverted to the same state as in "dev", just new "Add sequence of actions" button is added. New Sequence component is created. Sequence

surfingbytes avatar Apr 02 '24 12:04 surfingbytes

@surfingbytes can those three buttons be aligned in one line? Something like this: image

Misiu avatar Apr 02 '24 13:04 Misiu

I tried, but as it's 2 components, it is not possible. But it's kinda like Choose action, where it has another row for "add default actions": image

surfingbytes avatar Apr 02 '24 14:04 surfingbytes

Can you please check it again @karwosts ?

You're not waiting for me, you need to wait for one of the maintainers to bless this.

I tried, but as it's 2 components, it is not possible.

I won't opine on if this is better as one row or two, but I think this could be done by adding a <slot> for a third button in ha-automation-action.ts, and then filling that slot in ha-automation-action-parallel.ts

karwosts avatar Apr 07 '24 18:04 karwosts

I think this could be done by adding a <slot> for a third button in ha-automation-action.ts, and then filling that slot in ha-automation-action-parallel.ts

Good idea, thanks. Unfortunately it didn't work when I tried. Either I'm doing something wrong, or it has to do something with the fact that in this case, it would be parent adding elements to slot of it's child. The usage of slot I found in source code is always child adding elements to it's parent's slot.

Anyway, is having it in one line necessary? There are other components (Choose, If-Then) that has commands in 2 lines. If it can be left in 2 lines, is there anything else that needs to be done to get this approved?

surfingbytes avatar Apr 12 '24 09:04 surfingbytes

I don't know if it's necessary, I just replied out of curiousity and a learning exercise. Anyway I think it does work, I tried like this:

in ha-automation-action:

          <div class="buttons">
            <ha-button
              outlined
              .disabled=${this.disabled}
              .label=${this.hass.localize(
                "ui.panel.config.automation.editor.actions.add"
              )}
              @click=${this._addActionDialog}
            >
              <ha-svg-icon .path=${mdiPlus} slot="icon"></ha-svg-icon>
            </ha-button>
            <ha-button
              .disabled=${this.disabled}
              .label=${this.hass.localize(
                "ui.panel.config.automation.editor.actions.add_building_block"
              )}
              @click=${this._addActionBuildingBlockDialog}
            >
              <ha-svg-icon .path=${mdiPlus} slot="icon"></ha-svg-icon>
            </ha-button>
            <slot name="extrabutton"></slot>
          </div>

In ha-automation-action-parallel

  protected render() {
    const action = this.action;

    return html`
      <ha-automation-action
        .path=${[...(this.path ?? []), "parallel"]}
        .actions=${action.parallel}
        .disabled=${this.disabled}
        @value-changed=${this._actionsChanged}
        .hass=${this.hass}
      >
            <ha-button
              slot="extrabutton"
              label="Add Sequence"
              @click=${this._testbutton}
            >
              <ha-svg-icon .path=${mdiPlus} slot="icon"></ha-svg-icon>
            </ha-button>
      </ha-automation-action>
    `;
  }

  private _testbutton(ev: CustomEvent) {
    console.log("Pressed the button");
  }

Renders like: image

You don't have to do it this way, I just mentioned as you had said you were trying to put them on one line. Sorry review is taking a while, thanks for your patience and your contribution!

karwosts avatar Apr 12 '24 12:04 karwosts

@karwosts , thank you for the example with slot. However, that i exactly what I did (and you can see it in the last commit), but not a single thing changed in UI - it's still in 2 rows. And when I put "test" string into the slot in ha-automation-action, the "test" is shown on the first line. But button remains on the second one. Really strange, but I don't know what is wrong with it.

@Misiu is this requirement to get it approved? If not, can it be approved please?

surfingbytes avatar Apr 22 '24 16:04 surfingbytes

@Misiu is this requirement to get it approved? If not, can it be approved please?

Sorry for the late reply. I'm not the person making the approval decision, I'm just an occasional contributor. I suggested putting everything in a single row as it looks nicer and saves space. This was a suggestion, not an approval requirement, but I'm glad you addressed it. Nice job 👏🚀

Misiu avatar Apr 24 '24 10:04 Misiu

@matthiasdebaat , if this UI is ok, could you please approve it?

image

surfingbytes avatar May 01 '24 06:05 surfingbytes