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

Apply initial values to forms of device trigger extra fields #20555

Conversation

farmio
Copy link
Contributor

@farmio farmio commented Apr 19, 2024

Proposed change

This applies a schemas initial values for device trigger extra fields.

This can be tested with a core-branch adding a new device trigger for "Sun" with some dummy extra_fields
home-assistant/core#115846

Here the "bool" switch is "off", even though the default value is True.
Bildschirmfoto 2024-04-19 um 19 14 00


Here are some more selectors without default values with following configuration:

"extra_fields": vol.Schema(
    {
        vol.Required("bool", default=True): selector.BooleanSelector(),
        vol.Required("const", default=True): selector.ConstantSelector(
            selector.ConstantSelectorConfig(label="Constant", value=True)
        ),
        vol.Required("time"): selector.TimeSelector(
            selector.TimeSelectorConfig()
        ),
        vol.Required("datetime"): selector.DateTimeSelector(
            selector.DateTimeSelectorConfig()
        ),
        vol.Required("duration"): selector.DurationSelector(
            selector.DurationSelectorConfig(enable_day=False)
        ),
        vol.Required("number"): selector.NumberSelector(
            selector.NumberSelectorConfig(min=10, max=100)
        ),
        vol.Required("entity"): selector.EntitySelector(
            selector.EntitySelectorConfig(
                filter=selector.EntityFilterSelectorConfig(integration="sun")
            )
        ),
    }
)
previous with initial ha-form data
Bildschirmfoto 2024-04-20 um 11 20 28 Bildschirmfoto 2024-04-20 um 11 20 59

I'm not very familiar with ha-form so I'm not sure why it does ignore a default value of a schema item that looks like

{"selector": {"boolean": {}}, "name": "test", "required": true, "default": true}

when the key is not included in the data in

<ha-form
.hass=${this.hass}
.data=${this._extraFieldsData(this.trigger, this._capabilities)}
.schema=${this._capabilities.extra_fields}

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

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

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:

@karwosts
Copy link
Contributor

karwosts commented Apr 19, 2024

I'm not very familiar with ha-form so I'm not sure why it does ignore a default value of a schema item that looks like

I think I may have ran into this once before, I believe the ha-selector-boolean is missing placeholder support for rendering default: true. I did add this in another PR I was working on but it hasn't been merged yet.

This fix should cause the switch to appear as visually on when the key is missing from the data, but the field has a default of true.

https://github.com/home-assistant/frontend/pull/19718/files#diff-2991ed11d1b6a96a4567607ab82cc480b314844d8280150dac03480293c84229

Actually it looks like defaults are only used in forms when required: false, so maybe that is a different issue:

  .placeholder=${item.required ? "" : item.default}

@farmio
Copy link
Contributor Author

farmio commented Apr 20, 2024

Ah, placeholder could make sense here - so show this state but don't add it to data.
I don't think it should behave differently on required or optional in the Boolean case.

@farmio farmio changed the title Apply default values of device trigger extra fields Apply initial values to forms of device trigger extra fields Apr 20, 2024
Copy link
Member

@bramkragten bramkragten left a comment

Choose a reason for hiding this comment

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

I think this is correct, adding placeholders can potentially be done in a different PR.

@bramkragten bramkragten merged commit 141107f into home-assistant:dev Apr 22, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants