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

Mark more trigger fields as optional #10798

Merged
merged 1 commit into from
Dec 6, 2021

Conversation

spacegaier
Copy link
Member

@spacegaier spacegaier commented Dec 4, 2021

Breaking change

Proposed change

Marked some more fields as optional.

There are a few others such as numeric_state where at least one of "Above" or Below" is required. Perhaps we should add a hint for those as well)?

Something like: Above (this or "Below" is required)

@matthiasdebaat Do you think that makes sense or is just cluttering the UI? If the user does not provide either of them, we show an (slightly hard to read) error toast.

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 911d322 into home-assistant:dev Dec 6, 2021
@matthiasdebaat
Copy link
Collaborator

@matthiasdebaat Do you think ...

Hmm, didn't get notifications from PR's. Just changed the settings for this.

Marked some more fields as optional.

I like these text changes! Good to show what is optional, it's positive instead of showing things that are required.

There are a few others such as numeric_state where at least one of "Above" or Below" is required. Perhaps we should add a hint for those as well)?

Something like: Above (this or "Below" is required)

As this PR is now merged leaves it for now as is. I need to check the UI how it is right now.

@balloob balloob mentioned this pull request Dec 6, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 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.

Optional Fields in State Trigger Not Marked
4 participants