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

HelperText as a generic component #1820

Closed

Conversation

Pierstoval
Copy link
Contributor

@Pierstoval Pierstoval commented Oct 4, 2023

This feature is spread across several other components, and I had to use it on a custom project at some point, and I thought it would be a better option to be able to reuse the base Carbon feature directly as a Svelte component.

I ran yarn build:docs and it seems fine, and I updated all components using it so the behavior shouldn't change, since the HTML code behind is the same as before, so there is no breaking change.
I also ran yarn lint, and it updated some other files, sorry for that 😅.

I have questions though:

  • I'm still not sure about whether the helperText parameter should be flexible and injected either as an argument or as a <slot> element, but IMO having both available is fine, please tell me if it's ok.
  • Another thing I'm not 100% sure for now is the name: HelperText seems generic, but it's mostly used in Form components. Carbon design system initially shows this behavior only in forms. Should I therefore put it in the Form namespace? Rename it FormHelperText? Something else?

Side-note: The initial reason for this was that at some point I noticed that some form components didn't have the helperText feature (like Checkbox for instance). So if this is merged, I think it's going to be a bit simpler to add support for helperText to many other components 👌

@Pierstoval Pierstoval closed this Oct 17, 2023
@Pierstoval Pierstoval deleted the helpertext branch October 17, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant