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(forms): hide input control text under valid icon #1306

Merged
merged 4 commits into from
Oct 17, 2022

Conversation

julien-deramond
Copy link
Member

@julien-deramond julien-deramond commented Jun 10, 2022

Fixes #1305

This PR is a proposal to fix #1305 by:

  • adding the same padding-right rule than in Bootstrap
  • avoiding to display the valid icon for Quantity Selector (not enough space)

Dear reviewer please check all possible cases in forms when inputs are valid.

Live previews

@netlify
Copy link

netlify bot commented Jun 10, 2022

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 3090c1e
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/634d52540733ac000873e464
😎 Deploy Preview https://deploy-preview-1306--boosted.netlify.app/docs/5.2/components/tooltips
📱 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.

@julien-deramond julien-deramond marked this pull request as ready for review June 10, 2022 11:58
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

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

The actual implementation seems to be great, but here are few questions:

Color picker could be removed too ? Or maybe do it as Bs ?
image
image
image
image

Few points that could be handled in other PR('s):

  • We should remove .is-invalid l.249 in validation.md Handled with last merge of Bs
  • We should be careful for input[role="switch"][required]:not(:checked) invalid state (due to a filter):
    image

Copy link
Contributor

@MewenLeHo MewenLeHo left a comment

Choose a reason for hiding this comment

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

I think we still need the changes suggested by @louismaximepiton in his comment.
Or at least we should create a distinct issue for the problem raised by LM.

@julien-deramond julien-deramond force-pushed the main-jd-fix-input-valid-icon-overflow branch from ac4259b to a2456b4 Compare October 17, 2022 09:53
@julien-deramond
Copy link
Member Author

@MewenLeHo or @louismaximepiton Please check this new version including 87892ff which should fix .form-control-color and .form-check-input use cases 🤞

@MewenLeHo
Copy link
Contributor

@MewenLeHo or @louismaximepiton Please check this new version including 87892ff which should fix .form-control-color and .form-check-input use cases 🤞

LGTM 👌

@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@julien-deramond julien-deramond merged commit a64f2a1 into main Oct 17, 2022
@julien-deramond julien-deramond deleted the main-jd-fix-input-valid-icon-overflow branch October 17, 2022 13:07
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.

Text overflow not hidden with input text validation icons
4 participants