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

Prevent optional time fields in ha-service-control from auto-enabling #15124

Merged
merged 3 commits into from Jan 23, 2023

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Jan 17, 2023

Proposed change

Services with an optional time parameter (using ha-selector-time) do not behave correctly, in that every time the service is loaded in the UI editor, the time parameter is always set to enabled. Other optional parameters are always disabled by default.

This means that every time an automation is loaded in the UI, a time: "00:00:00" will be added to a service that has an optional time parameter, even if user does not desire that parameter to be defined. Deleting it, and refreshing the page, will bring it right back again.

This occurs because when time is initialized (undefined by default), if localization requires an am/pm selector, it will instance the selector, default to AM, and then fire a "valueChanged" event with the value:

hours: NaN
minutes: NaN
seconds: NaN
ampm: "AM"

When ha-time-input gets this event, it coerces it into a value of "00:00:00" and itself fires a value changed up to its parent selector and service control. This event tricks the service control into thinking the time parameter exists with a value of 12am, and sets that parameter to enabled.

Suggest fix this in ha-time-input by treating valueChanged events with hours, minutes, and seconds as NaN as an undefined value, instead of "00:00:00". Then the time key remains correctly undefined after initialization, if the value is not already present in the yaml.

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:

@bramkragten bramkragten merged commit 38c1112 into home-assistant:dev Jan 23, 2023
@karwosts karwosts deleted the optional-time-selector-fix branch January 23, 2023 19:01
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2024
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.

Sonos update alarm automation adds a default time if none is given
2 participants