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

feat(forms): change rendering of help texts based on UI Kit v5 #1779

Merged
merged 11 commits into from
Apr 5, 2023

Conversation

hannahiss
Copy link
Member

@hannahiss hannahiss commented Jan 13, 2023

Related issues

Fixes #1767

Description

Set new styles for form-text (help-text) for UI kit V5:

  • set font-size to 14px
  • set font-weight to bold
  • set font-color to #666666
  • set line-height to 16px

Before:
image

After:
image

Design:
image

There is a tiny shift of 1px between the text and the input compared to the design, like for error message, we'll have to validate this in design review (or not)

Motivation & Context

Apply changes in design after DSM V5 release

Types of change

  • Bug fix (non-breaking which fixes an issues)

Live previews

Checklist

Contribution

Accessibility

  • My change follows accessibility good practices; I have at least run axe

Design

  • My change respects the design guidelines defined in Orange Design System
  • My change is compatible with responsive display

Development

  • My change follows the developer guide
  • I have added JavaScript unit tests to cover my changes

Documentation

  • My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

  • My change introduces changes to the migration guide
  • (NA) My new component is added in Storybook
  • (NA) My new component is compatible with RTL
  • Manually run BrowserStack tests
  • Manually test browser compatibility with BrowserStack (Chrome >= 60, Firefox >= 60 (+ ESR), Edge, Safari >= 12, iOS Safari, Chrome & Firefox on Android)
  • Code review
  • Design review
  • A11y review

After the merge

@netlify
Copy link

netlify bot commented Jan 13, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 2f13c95
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/642d1ac4b151ad00085d2305
😎 Deploy Preview https://deploy-preview-1779--boosted.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.

@sonarcloud
Copy link

sonarcloud bot commented Jan 13, 2023

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

@hannahiss hannahiss marked this pull request as ready for review January 13, 2023 14:11
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.

I can see a shift of 1px between the text and the input compared to the design, but let's keep it for the design review!

# Conflicts:
#	site/content/docs/5.3/migration.md
@hannahiss
Copy link
Member Author

I can see a shift of 1px between the text and the input compared to the design, but let's keep it for the design review!

Yes, you're right, but it is consistent with error messages though...
image

Still awaiting for design review, we'll see what Franco will say (card "Dev. Design review >> Forms Help text")

@Franco-Riccitelli
Copy link

Looks good. As you mentioned in your comment in this pull request, there is a 1 px difference in the positioning to the visual specification.
 If it is possible to move the Help text 1 px down, that would be perfect. Thanks

# Conflicts:
#	site/content/docs/5.3/migration.md
@hannahiss
Copy link
Member Author

From DSM or Zeplin, it is asked 6px of top margin, but it makes 10px from the bottom of the input and the top of the uppercase letters, see screenshots below:

DSM (Validation error message):
image

Zeplin (Validation error message)
image

Zeplin (Form text):
image


In Boosted, we have currently (not changed), for error message 9px:
image

So, when I implement Help text, I have the same than for error, 9px:
image


So, does it mean that error messages should be 1 px lower too? @julien-deramond @Franco-Riccitelli

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Just some details to change (see suggestions).
IMO we can merge this PR as is because it is consistent with the alignment of valid/invalid messages. The alignment issue observed should be tackled in a separate issue.

scss/_variables.scss Outdated Show resolved Hide resolved
scss/_variables.scss Outdated Show resolved Hide resolved
hannahiss and others added 2 commits April 4, 2023 11:43
Co-authored-by: Julien Déramond <[email protected]>
Co-authored-by: Julien Déramond <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Apr 5, 2023

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 changed the title Change rendering of help texts to conform to design feat(forms): change rendering of help texts based on UI Kit v5 Apr 5, 2023
@julien-deramond julien-deramond merged commit 7251b6f into main Apr 5, 2023
@julien-deramond julien-deramond deleted the main-his-help-text branch April 5, 2023 06:57
@julien-deramond julien-deramond mentioned this pull request Apr 5, 2023
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.

Change rendering of help texts
4 participants