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

Confirmation Dialog #4053

Merged
merged 13 commits into from
Oct 22, 2019
Merged

Confirmation Dialog #4053

merged 13 commits into from
Oct 22, 2019

Conversation

timmo001
Copy link
Contributor

@timmo001 timmo001 commented Oct 18, 2019

Description

Adds a confirmation dialog which can replace all the browser-based window.confirm which helps the ui stays the same throughout.

image
image
image
image
image
image
image

Added to

  • Call service (restart and stop HA)
  • Integrations (delete entry)
  • Entity Registry (delete entry)
  • Views (deletion)
  • Cards (deletion)

More to add to, but this is a good start

@iantrich
Copy link
Member

🎉 Nice!

@timmo001 timmo001 marked this pull request as ready for review October 18, 2019 18:45
@timmo001
Copy link
Contributor Author

OK I think I've covered the most of the app. I'd like to do another for alerts too but will wait until this one is approved and in then create a new dialog for those. 😃

}

private async _dismiss(): Promise<void> {
this._params = undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Should probably add a dismiss callback for the future.

Comment on lines 154 to 164
deleteConfigEntry(this.hass, this.configEntryId).then((result) => {
fireEvent(this, "hass-reload-entries");
if (result.require_restart) {
alert(
this.hass.localize(
"ui.panel.config.integrations.config_entry.restart_confirm"
)
);
}
navigate(this, "/config/integrations/dashboard", true);
}),
Copy link
Member

Choose a reason for hiding this comment

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

Move this to a separate function like you did for the rest:
confirm: () => this._deleteConfigEntry()

export interface ConfirmationDialogParams {
title?: string;
text: string;
confirm: () => void;
Copy link
Member

Choose a reason for hiding this comment

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

For the future, we should add a dismiss callback, and the ability to overwrite the OK and 'Cancel' buttons text.

@timmo001
Copy link
Contributor Author

@bramkragten are you implementing these changes? I was about to carry them out but can let you finish off this PR if you like 😃

@bramkragten
Copy link
Member

@bramkragten are you implementing these changes? I was about to carry them out but can let you finish off this PR if you like 😃

Yeah wanted to get this in before I make a release 😄 sorry

@timmo001
Copy link
Contributor Author

@bramkragten are you implementing these changes? I was about to carry them out but can let you finish off this PR if you like smiley

Yeah wanted to get this in before I make a release sorry

Go ahead. Cheers! 🍻

@bramkragten bramkragten merged commit bb73039 into home-assistant:dev Oct 22, 2019
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.

None yet

4 participants