-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Continue on error UI #28095
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
base: dev
Are you sure you want to change the base?
Continue on error UI #28095
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import type { NonConditionAction } from "../../../../../src/data/script"; | ||
|
|
||
| /** | ||
| * Helper function that mirrors the toggle logic from ha-automation-action-row.ts | ||
| * This tests the core logic without needing to instantiate the full component. | ||
| */ | ||
|
Comment on lines
+4
to
+7
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this test is 100% synthetic, it doesn't test any real code from the design? I wouldn't think we would want that, doesn't seem useful to me. Otherwise it provides no benefit going forward.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'll drop that commit |
||
| function toggleContinueOnError(action: NonConditionAction): NonConditionAction { | ||
| const continueOnError = !(action.continue_on_error ?? false); | ||
| if (continueOnError) { | ||
| return { ...action, continue_on_error: true }; | ||
| } | ||
| const result = { ...action }; | ||
| delete result.continue_on_error; | ||
| return result; | ||
| } | ||
|
|
||
| describe("continue_on_error toggle", () => { | ||
| it("should enable continue_on_error when currently undefined", () => { | ||
| const action: NonConditionAction = { | ||
| action: "light.turn_on", | ||
| target: { entity_id: "light.bedroom" }, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBe(true); | ||
| expect(result.action).toBe("light.turn_on"); | ||
| }); | ||
|
|
||
| it("should enable continue_on_error when currently false", () => { | ||
| const action: NonConditionAction = { | ||
| action: "light.turn_on", | ||
| continue_on_error: false, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBe(true); | ||
| }); | ||
|
|
||
| it("should remove continue_on_error when currently true", () => { | ||
| const action: NonConditionAction = { | ||
| action: "light.turn_on", | ||
| target: { entity_id: "light.bedroom" }, | ||
| continue_on_error: true, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBeUndefined(); | ||
| expect(result.action).toBe("light.turn_on"); | ||
| expect(result.target).toEqual({ entity_id: "light.bedroom" }); | ||
| }); | ||
|
|
||
| it("should preserve other action properties when toggling on", () => { | ||
| const action: NonConditionAction = { | ||
| alias: "Turn on bedroom light", | ||
| action: "light.turn_on", | ||
| target: { entity_id: "light.bedroom" }, | ||
| data: { brightness: 255 }, | ||
| enabled: true, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBe(true); | ||
| expect(result.alias).toBe("Turn on bedroom light"); | ||
| expect(result.action).toBe("light.turn_on"); | ||
| expect(result.target).toEqual({ entity_id: "light.bedroom" }); | ||
| expect(result.data).toEqual({ brightness: 255 }); | ||
| expect(result.enabled).toBe(true); | ||
| }); | ||
|
|
||
| it("should preserve other action properties when toggling off", () => { | ||
| const action: NonConditionAction = { | ||
| alias: "Turn on bedroom light", | ||
| action: "light.turn_on", | ||
| target: { entity_id: "light.bedroom" }, | ||
| continue_on_error: true, | ||
| enabled: false, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBeUndefined(); | ||
| expect(result.alias).toBe("Turn on bedroom light"); | ||
| expect(result.enabled).toBe(false); | ||
| }); | ||
|
|
||
| it("should work with delay action", () => { | ||
| const action: NonConditionAction = { | ||
| delay: "00:00:05", | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBe(true); | ||
| expect((result as any).delay).toBe("00:00:05"); | ||
| }); | ||
|
|
||
| it("should work with event action", () => { | ||
| const action: NonConditionAction = { | ||
| event: "custom_event", | ||
| event_data: { key: "value" }, | ||
| }; | ||
|
|
||
| const result = toggleContinueOnError(action); | ||
|
|
||
| expect(result.continue_on_error).toBe(true); | ||
| expect((result as any).event).toBe("custom_event"); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel like with just the icon changing between two very similar variants, and the label staying static, that I would really be able to tell what state this was in.
A checked/unchecked checkbox would be more intuitive maybe?
Other toggles also change their verbiage between state e.g. Enable / Disable, but I don't know if "Enable Continue on Error / Disable Continue on Error" is uncomfortably long for the dropdown.
I would maybe wait for UX review before making any changes though.