Skip to content

Merge Confirmation Dialogs into Alert, Confirmation or Prompt#4114

Merged
bramkragten merged 10 commits into
home-assistant:devfrom
timmo001:alert-dialog
Jan 25, 2020
Merged

Merge Confirmation Dialogs into Alert, Confirmation or Prompt#4114
bramkragten merged 10 commits into
home-assistant:devfrom
timmo001:alert-dialog

Conversation

@timmo001
Copy link
Copy Markdown
Member

@timmo001 timmo001 commented Oct 23, 2019

Description

Adds alert dialog and replaces the confirmation dialog so that prompts and alerts can be called in the same component. Also adds prompts to more sections of the UI.

Issue this references

Closes #4103

Screenshots

Dev tools - States (no entity_id):

image

Dev tools - Events (no event type)

image

Automation - Delete:

image

@timmo001 timmo001 marked this pull request as ready for review October 23, 2019 17:49
@timmo001
Copy link
Copy Markdown
Member Author

There's a good few, but I'll leave it at that for now. 😃

Comment thread src/dialogs/alert/dialog-alert.ts Outdated
@bramkragten
Copy link
Copy Markdown
Member

I wonder if we can't combine the dialogs, the only real difference is the cancel button?

@bramkragten
Copy link
Copy Markdown
Member

And later also add a prompt

@timmo001
Copy link
Copy Markdown
Member Author

I guess so. I'll take the android naming of an AlertDialog since that allows 1+ options

@timmo001 timmo001 changed the title Add Alert Dialog Merge Confirmation Dialogs into Alert Dialogs Oct 26, 2019
@timmo001
Copy link
Copy Markdown
Member Author

timmo001 commented Oct 26, 2019

Working correctly now with single component.

Cast integration deletion (good test as usually requires restart):
image

No cancel or title on secondary dialog:
image

Restart:
image

Invalid event type:
image

Invalid entity:
image

@timmo001 timmo001 requested a review from bramkragten October 26, 2019 16:02
@bramkragten
Copy link
Copy Markdown
Member

With prompt I meant this: https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt

@bramkragten
Copy link
Copy Markdown
Member

So we would have: alert, confirm and prompt

@bramkragten
Copy link
Copy Markdown
Member

And I suggest we make 3 different helpers for the same dialog:
showAlert, showConfirm and showPrompt

Comment thread src/dialogs/generic/show-dialog-generic.ts Outdated
Comment thread src/dialogs/generic/dialog-generic.ts Outdated
Comment thread src/dialogs/generic/show-dialog-generic.ts Outdated
@timmo001
Copy link
Copy Markdown
Member Author

Sorry should have set this to WIP. Still adding and changing, just pushing as I go 😃

@timmo001 timmo001 changed the title Merge Confirmation Dialogs into Alert Dialogs WIP: Merge Confirmation Dialogs into Alert Dialogs Nov 12, 2019
@timmo001
Copy link
Copy Markdown
Member Author

Here's the prompt in action:

image

image
image
image

@timmo001 timmo001 changed the title WIP: Merge Confirmation Dialogs into Alert Dialogs Merge Confirmation Dialogs into Alert Dialogs Nov 12, 2019
@timmo001 timmo001 changed the title Merge Confirmation Dialogs into Alert Dialogs Merge Confirmation Dialogs into Generic Alert Dialog for Alert, Confirmation and Prompt Nov 12, 2019
@timmo001 timmo001 changed the title Merge Confirmation Dialogs into Generic Alert Dialog for Alert, Confirmation and Prompt Merge Confirmation Dialogs into Generic Alert Dialog Nov 12, 2019
@timmo001 timmo001 changed the title Merge Confirmation Dialogs into Generic Alert Dialog Merge Confirmation Dialogs into Generic Dialog Nov 12, 2019
@timmo001
Copy link
Copy Markdown
Member Author

OK, let me know what you think

@timmo001 timmo001 requested a review from bramkragten November 12, 2019 14:23
Comment thread src/dialogs/generic/dialog-box.ts
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/show-dialog-box.ts
@timmo001 timmo001 changed the title Merge Confirmation Dialogs into Generic Dialog or Prompt Merge Confirmation Dialogs into Alert, Confirmation or Prompt Jan 23, 2020
@timmo001 timmo001 requested a review from bramkragten January 23, 2020 21:33
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts Outdated
`
: this._params.text
? html`
<p>${this._params.text}</p>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can display text when it is a prompt?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move this above since a prompt might want an explanation above the input

Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/dialog-box.ts Outdated
Copy link
Copy Markdown
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK to merge after comments are addressed.

timmo001 and others added 2 commits January 24, 2020 13:56
Co-Authored-By: Bram Kragten <mail@bramkragten.nl>
@timmo001
Copy link
Copy Markdown
Member Author

Commited suggestions. I'll take a proper look when I get home and address the rest later as I'm remote at the moment

@timmo001
Copy link
Copy Markdown
Member Author

OK, I think that's everything

Comment thread src/dialogs/generic/dialog-box.ts Outdated
Comment thread src/dialogs/generic/show-dialog-box.ts Outdated
Comment thread src/panels/config/users/ha-user-editor.ts Outdated
timmo001 and others added 2 commits January 25, 2020 14:00
Co-Authored-By: Bram Kragten <mail@bramkragten.nl>
Co-Authored-By: Bram Kragten <mail@bramkragten.nl>
@bramkragten bramkragten merged commit 15be168 into home-assistant:dev Jan 25, 2020
@timmo001 timmo001 deleted the alert-dialog branch January 25, 2020 17:25
@bramkragten bramkragten mentioned this pull request Jan 29, 2020
@timmo001 timmo001 mentioned this pull request Feb 2, 2020
8 tasks
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a generic alert dialog using built in solution instead of browser alert

4 participants