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

Multi textfield helper #23649

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Jan 8, 2025

Proposed change

Add helper text, and add label to ADD button. Sometimes there is no textbox visible, so the label would not otherwise be visible.

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:

@karwosts karwosts closed this Jan 8, 2025
@karwosts karwosts reopened this Jan 8, 2025
@karwosts karwosts closed this Jan 8, 2025
@karwosts karwosts reopened this Jan 8, 2025
@karwosts karwosts closed this Jan 8, 2025
@karwosts karwosts reopened this Jan 8, 2025
@piitaya piitaya added the Needs UX Pull requests requiring a review from the Home Assistant design team label Jan 9, 2025
@piitaya
Copy link
Member

piitaya commented Jan 9, 2025

The helper seems a little out of place because of the button. I will ask UX team to know how we can fix that.

@marcinbauer85
Copy link
Contributor

@karwosts I've made a design that addresses this PR, along with some other much needed fixes for this whole dialog. Feel free to include it also or not.

https://www.figma.com/design/Kmn09N7TCO1gaXlDJLinfn/PDX-1896-Helpers%3A-History-Stats-fix?node-id=2-130&t=1sZ6yfBcwFWheKUQ-11

@karwosts
Copy link
Contributor Author

karwosts commented Jan 9, 2025

@marcinbauer85 - This bug is not specifically for history stats, but regarding the lack of helper text in any config flow for this type of selector.

This form in my screenshot is not specific to history stats, it is just an example of a config flow form. You design omits helper text, which I guess could be fine for history stats, but it doesn't indicate what to do for integrations that do include helper text. It needs to be put somewhere.

Most of these changes you request would therefore be ha/core changes to the config flow schema for history_stats, which is out of scope for this PR.

@marcinbauer85
Copy link
Contributor

@karwosts Thanks for letting me know. The bugs are just something I've noticed and can be addressed by a seperate PR in the future. Helper text should be used only when validation is triggered on that specific selector, so in this case if triggered it would show up under the selector itself, not at the end, or under the "+ADD STATE") button.

@karwosts
Copy link
Contributor Author

Helper text should be used only when validation is triggered on that specific selector, so in this case if triggered it would show up under the selector itself, not at the end, or under the "+ADD STATE") button.

Sorry, this is hard for me to believe. Every other selector shows the description field. You're saying just for multiple-textfield selectors you don't want to show it? Or you want it to be removed for every selector from every integration? Not every field may even have possible validation, it may just be freeform input that needs explanation.

@marcinbauer85
Copy link
Contributor

For now please just align the "Add state" button to the left. In the future we'll tackle other UX issues of this helper and others.

@karwosts
Copy link
Contributor Author

Thank you Marcin, it has been left aligned.

@karwosts karwosts removed the Needs UX Pull requests requiring a review from the Home Assistant design team label Jan 17, 2025
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.

Multi-Text input shows neither name nor description in config flow
4 participants