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

Add reset all config for zwave node, Show default value per parameter #21894

Closed

Conversation

wendevlin
Copy link
Contributor

@wendevlin wendevlin commented Sep 5, 2024

Breaking change

Proposed change

Add a reset all config parameter functionality at the bottom of the zwave node configration panel.
image

Add the default value to every parameter:
image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • 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

Checklist

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

@wendevlin
Copy link
Contributor Author

Still in draft, because core isn't ready yet.

Copy link
Member

@piitaya piitaya left a comment

Choose a reason for hiding this comment

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

Nice 🙂 Feature-wise it's look ok for me!

I only have feedback about code style.
For dialog, we don't usually add ha-dialog in the render function.
For your use-case, you have two choices :

  1. use generic dialog (e.g. showConfirmationDialog)
This will open a confirmation dialog and return a promise with a boolean.

For example : 
```ts
 const confirm = await showConfirmationDialog(this, {
    title: this.hass.localize(
      "ui.panel.config.category.editor.confirm_delete"
    ),
    text: this.hass.localize(
      "ui.panel.config.category.editor.confirm_delete_text"
    ),
    confirmText: this.hass.localize("ui.common.delete"),
    destructive: true,
  });
  1. build your own dialog
    For example : https://github.com/home-assistant/frontend/blob/dev/src/dialogs/restart/show-dialog-restart.ts

If you need more logic than just text and confirmation, you can do something similar as you did but in separate files so the dialog can be lazy loaded when needed.

For this feature, I would keep it simple and use a confirmation dialog with a snack message for loading, success or error.

src/translations/en.json Outdated Show resolved Hide resolved
wendevlin and others added 2 commits September 9, 2024 11:25
Copy link
Contributor

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

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

From the Z-Wave perspective the frontend looks good. Just needs to actually do something when pressing the reset button.

@wendevlin wendevlin changed the title Add reset all config for zwave node Add reset all config for zwave node, Show default value per parameter Sep 12, 2024
Copy link
Member

@piitaya piitaya left a comment

Choose a reason for hiding this comment

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

Seems good to me ! I only put few suggestions to improve the dialog 🙂

@bramkragten bramkragten added wait for backend Backend Change Required Requires a Backend Core Code Change labels Sep 12, 2024
@wendevlin
Copy link
Contributor Author

@wendevlin wendevlin closed this Sep 16, 2024
@wendevlin wendevlin deleted the zwave-device-restore-config branch September 16, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants