tremorlabs / tremor

React components to build charts and dashboards
https://tremor.so
Apache License 2.0
15.66k stars 452 forks source link

fix: selectbox missing required name #729

Closed jzfrank closed 4 months ago

jzfrank commented 8 months ago

Description Add name and required props to Select, MultiSelect, SearchSelect components.

This is for consistency, for users who stick with form to submit data.

Currently, the fix is W.I.P., as it has not replicated the effect of native input's required behavior: i.e. preventing form from submitting if required input element is not filled.

Related issue(s)

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

If yes, please describe the impact and migration path for existing applications:

How has This been tested? In Storybook SelectWithForm, MultiSelectWithForm, SearchSelectWithForm. After selecting, click submit, it will open a page that the form requests.

Screenshots (if appropriate): e.g. MultiSelect form

image

After clicking submit ->

image

The PR fulfills these requirements:

christopherkindl commented 8 months ago

Hi @jzfrank, works great - just tested the submission also with a third party form service and works fine. The only thing missing now would be error and errorMessage as used in other input elements.

Thanks for the great work again!

jzfrank commented 8 months ago

Just checked @mbauchet edits, it seems that the form warning when no element selected is solved. Thanks and clever trick!

@christopherkindl @severinlandolt Regarding the error and errorMessage, can you elaborate why we need these fields for a Select element and use scenarios? Thanks :)

christopherkindl commented 8 months ago

@jzfrank If those components are used in a form context, which is common for form elements. E.g. "make a selection to continue with the process"

jzfrank commented 8 months ago

@christopherkindl I think I get the point, but not sure if completely. From my understanding, now we support:

Based on that interpretation, I just added error and errorMessage, similar to what we do for BaseInput. Let me know if they are as expected :)

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tremor-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 23, 2024 3:34am
severinlandolt commented 4 months ago

@jzfrank Congrats, the PR is now in beta!

github-actions[bot] commented 4 months ago

:tada: This PR is included in version 3.13.2-beta-select.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 4 months ago

:tada: This PR is included in version 3.13.4-beta-select.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: