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

Use service name in action confirmation popup #8493

Merged
merged 6 commits into from
Mar 31, 2021

Conversation

spacegaier
Copy link
Member

@spacegaier spacegaier commented Feb 28, 2021

Breaking change

Proposed change

If the action is call-service we now attempt to look-up the service name and show that in the confirmation popup.

Not all services in the backend provide a name currently (I created core PR home-assistant/core#47204 to fix that for HA scripts and Python scripts), but that is not an issue. If there is no service name, we fallback to the current generic handling.

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:

Comment on lines +70 to +73
hass.localize(
"ui.panel.lovelace.editor.action-editor.actions." +
actionConfig.action
) ||
Copy link
Member

Choose a reason for hiding this comment

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

We still want to show this part I think, just add what service will be called?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think many names assume that they will be used together with the domain/integration name, and not stand alone?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the end-user does not care if it is technically a "service" or not that is being called. So I opted only for the name.

I agree reg. the domain part. Need to see how to best include that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bramkragten I attempted to take over the logic that is also used in the ha-service-picker for the domain. The problem is that I cannot get the translations to get loaded. So the first time the popup gets shown, the domain is not translated, the second time around it is.

I attempted to manually force the "title" loading via hass.loadBackendTranslation("title"); but I did not have any effect.

=> Any hints / tips?

Copy link
Member

Choose a reason for hiding this comment

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

Probably a timing issue then? Do you await it? And use its result as input for domainToName?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am sure I did, but apparently both individually (either await or local var) and not combined (await + local var). Tried that now, and seems to work:

image

@spacegaier spacegaier merged commit 20858db into home-assistant:dev Mar 31, 2021
@spacegaier spacegaier deleted the issue-8492 branch March 31, 2021 12:12
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2021
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.

button confirmation question asking about the service, not the name
3 participants