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

fix(text-input): add class for inline validation #12231

Merged
merged 10 commits into from
Oct 21, 2022

Conversation

aledavila
Copy link
Contributor

Closes #10234

Fix validation on inline variant

Changelog

New

  • added new class to target inline validation

Testing / Reviewing

Make sure validation props still work. Make sure validation props inline are showing.

@aledavila aledavila requested a review from a team as a code owner October 4, 2022 18:59
@netlify
Copy link

netlify bot commented Oct 4, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit ab722a0
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6352eb595b30c00009430b44
😎 Deploy Preview https://deploy-preview-12231--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 4, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ab722a0
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6352eb590811d10008410ee3
😎 Deploy Preview https://deploy-preview-12231--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@tw15egan
Copy link
Member

tw15egan commented Oct 5, 2022

Screen Shot 2022-10-05 at 8 58 37 AM

Had a few questions:

  • Should the invalid text be invalid colored?
  • The label wraps into two lines even with just two words, but I think this may be a storybook-specific issue (maybe we should increase the width?)

@aledavila
Copy link
Contributor Author

@tw15egan oh yes i updated the class but not in the css. The wrapping is due to the width in the story being at 300. But if you go to the "playgroundWidth" control in there you can make that larger and it fits well. I think it inline being is a difficult for invalid and warn texts since it's such a small area. People will need to keep it short and sweet.

@jnm2377 jnm2377 requested review from a team and thyhmdo and removed request for a team October 5, 2022 15:19
@aledavila
Copy link
Contributor Author

@jnm2377 @tw15egan updated

@thyhmdo
Copy link
Member

thyhmdo commented Oct 18, 2022

Small ask @aledavila but can we change that text from "Invalid text" to "Error message goes here"? I think we'll try to be more consistent with our guidance languages and since we have this issue open so I just wanted to add that.

@aledavila
Copy link
Contributor Author

@thyhmdo yup I'll change that no problem

@aledavila
Copy link
Contributor Author

@thyhmdo updated now

Copy link
Member

@thyhmdo thyhmdo left a comment

Choose a reason for hiding this comment

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

thanks @aledavila looks good

@kodiakhq
Copy link
Contributor

kodiakhq bot commented Oct 19, 2022

This PR currently has a merge conflict. Please resolve this and then re-add the status: ready to merge 🎉 label.

@sstrubberg sstrubberg merged commit 5e676fd into carbon-design-system:main Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Question]: Should TextInput inline invalidMessage appear somewhere in the DOM?
5 participants