feat:text field wrapper component

MartinPiq commented 3 months ago

Describe pull-request

New text field wrapper component. Instead of current usage where we have a custom text field with tds events this one is a wrapper around a input element. All attributes that belong to input element have been removed from the text field wrapper and are directly added to the input element. This probably still needs some refinement/improvement.

Usage would look something like this.

<tds-text-field-wrapper size="lg" state="default" label="Label" label-position="no label">
    <input type="text" placeholder="Placeholder" />
</tds-text-field-wrapper>

This has been heavily inspired by Porsche Design System components which implement this functionality. The utility code added has mostly been copied from there repository.

Issue Linking:

No issue: Describe the problem being solved.
Current text field component is not always easy to integrate into different solutions since it is a custom component. Using wrappers for elements handling input allows them to be more framework agnostic and avoids the need to add custom functionality to integrate them in different frameworks.

In my opinion this should be the standard for all elements handling user input. (text field, textarea, checkbox, etc...)

How to test

Try it out in the storybook. I will probably have missed a few scenarios or features from the normal tds text field so it requires a proper look through to verify it has all the text field functionality.

Checklist before submission

[x] I have added unit tests for my changes (if applicable)
[x] All existing tests pass
[ ] I have updated the documentation (if applicable) Need to add proper documentation for how to use this component and it's differences from the normal text field. Wanted to create pull request for visibility and invite collaboration.

Suggested test steps

[ ] Browser testing (Chrome, Safari, Firefox)
[ ] Keyboard operability
[ ] Interactive elements have labels.
[ ] Storybook controls
[ ] Design/controls/props is aligned with other components
[ ] Dark/light mode and variants
[ ] Input fields – values should be displayed properly
[ ] Events

aws-amplify-eu-north-1[bot] commented 3 months ago

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-673.d3fazya28914g3.amplifyapp.com

theJohnnyMe commented 2 months ago

From discussion Johan and I had: What if we have that is "smart" so based on type it provides the right styling for it? So one component that works OK with all form elements. Food for discussion in August.

MartinPiq commented 2 months ago

From discussion Johan and I had: What if we have that is "smart" so based on type it provides the right styling for it? So one component that works OK with all form elements. Food for discussion in August.

I think the idea is interesting and worth discussing but that it will probably result in a really complex wrapper with tons of conditional logic. Adding more functionality and features without affecting existing ones will become more and more challenging as time goes on. There are already properties used for our current tegel components that wouldn't make sense to have on every wrapper, one example being label position.

mJarsater commented 2 months ago

Just asking. Instead of this being another component, what if instead it is the text-field component? And we'd do the same for all relevant input elements

MartinPiq commented 2 months ago

Just asking. Instead of this being another component, what if instead it is the text-field component? And we'd do the same for all relevant input elements

I have discussed this with a few Tegel team members and I think we are all mostly in agreement that this is the direction we want to move in. One idea is to initially release them as alpha/beta components, then move towards tagging the non wrapper input elements as deprecated and then remove them in some future.

mJarsater commented 2 months ago

Just asking. Instead of this being another component, what if instead it is the text-field component? And we'd do the same for all relevant input elements

I have discussed this with a few Tegel team members and I think we are all mostly in agreement that this is the direction we want to move in. One idea is to initially release them as alpha/beta components, then move towards tagging the non wrapper input elements as deprecated and then remove them in some future.

Sounds like a good plan. I would just really validate that it all works as expected on all input components that are relevant.

If this is the case, then this "wrapper" component could just be a <label> that wrapps the input for a11y

sonarcloud[bot] commented 2 weeks ago

Quality Gate failed

Failed conditions
11.4% Duplication on New Code (required ≤ 3%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

scania-digital-design-system / tegel