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.

Sign up for GitHub

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

Jump to bottom

Forms>Validation : Add a link to "Terms and conditions" checkboxes #2002

Merged
merged 7 commits into from
Jul 5, 2023
Merged

Forms>Validation : Add a link to "Terms and conditions" checkboxes #2002

merged 7 commits into from
Jul 5, 2023

Conversation

isabellechanclou
Copy link
Member

@isabellechanclou isabellechanclou commented Apr 20, 2023 •
edited by MewenLeHo
Loading

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

#1715

Description

Designers said that the checkbox label "Agree Terms and Conditions" should be a link towards a "Terms and Conditions" page that the user is supposed to read before checking the box.

As no design specification is available yet, a design review is to be asked.

Motivation & Context

This changed was required by designers.

Types of change

  • New feature (non-breaking change which adds functionality)

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
  • I have added SCSS 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
  • My new component is added in Storybook
  • 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

@isabellechanclou isabellechanclou added v5 docs Improvements or additions to documentation labels Apr 20, 2023
@isabellechanclou isabellechanclou self-assigned this Apr 20, 2023
@netlify
Copy link

netlify bot commented Apr 20, 2023 •
edited
Loading

✅ Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 5271fdb
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/64a523bef084a70008dfa744
😎 Deploy Preview https://deploy-preview-2002--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 configuration.

@isabellechanclou
Copy link
Member Author

I did the fix as asked by design that is with the entire label as a link (therefore visually underlined) and with the following interactive behavior: when clicking on the label, a new page opens and the checkbox doesn't change state.
Now, it might be a bit confusing for users to have the entire checkbox label underlined, knowing that quite often, to check/uncheck a checkbox, users have 2 possibilities: clicking on the box or clicking on the label.

So to avoid this possible confusion, we also can do it that way as illustrated below: we place only the "terms and conditions" words in the link. It's know clearer to know that there is a link toward a new content. In addition, to tick/untick the box, users still have to possibilities: clicking on the box or clicking on the "Agree to" part of the label.
⚠️ This last proposal will have to be seen with designers and they will have to tell us which version they want to keep.

image

@isabellechanclou isabellechanclou linked an issue Apr 21, 2023 that may be closed by this pull request
Docs > Component > Forms > Validation: Fix "Agree to terms and conditions" checkbox label #1715
Closed
3 tasks
@MewenLeHo MewenLeHo self-requested a review April 24, 2023 06:55
MewenLeHo
MewenLeHo reviewed Apr 24, 2023
View reviewed changes
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.

Some a11y fixes needed.

Regarding the design guideline, if this is truly the rule then the rule must be clearly written somewhere.

@isabellechanclou
Copy link
Member Author

isabellechanclou commented Apr 26, 2023 •
edited
Loading

It has been decided during a spec web meeting (on April 24th 2023):

  • to replace "Agree to terms and conditions" by "By checking this box, I agree to the terms and conditions"
  • to make "terms and conditions" a link
  • when the user click on this link, a new browser tab opens containing these terms and conditions
  • to accept in the code href=""instead of href="#" if the new tab doesn't open when href="#" (still the case)
  • to have the checkbox be checked/unchecked when clicking on the checkbox itself or on the text "Agree to terms and conditions" by "By checking this box, I agree to the" that is not in the link.
  • to delete the extra space between the end of the label and the required field symbol (*)

The result should therefore be:
image

@MewenLeHo MewenLeHo self-requested a review April 26, 2023 13:10
@MewenLeHo MewenLeHo self-requested a review May 12, 2023 12:12
@sonarcloud
Copy link

sonarcloud bot commented May 12, 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
No Duplication information No Duplication information

MewenLeHo
MewenLeHo approved these changes May 12, 2023
View reviewed changes
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.

LGTM 🚀

@Franco-Riccitelli
Copy link

This looks good. No issues. Thanks.

Aniort
Aniort approved these changes Jul 4, 2023
View reviewed changes
Copy link
Contributor

@Aniort Aniort left a comment

Choose a reason for hiding this comment

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

it's ok !

@isabellechanclou @julien-deramond
Add a link to "Terms and conditions" checkboxes
a4b55ac 
Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Add aria-label on Terms and conditions link
b899f8f 
Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Fixes after web spec review meeting
7f61a13 
Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Remove margin before the required symbol at the end of the label "...…
f1a3fa5 
… terms and conditions"

Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Add "Boosted mod" missing comment
6ace7f0 
Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Fix extra space by putting on 1 single line the entire "agree to term…
0606f91 
…s and conditions" checkbox label + reintroducing margin-left in css.

Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@isabellechanclou @julien-deramond
Delete extra spaces
5271fdb 
Signed-off-by: Isabelle Chanclou <isabelle.chanclou@orange.com>
@julien-deramond julien-deramond force-pushed the main-ic-add-link-to-agree-terms-and-condition-checkbox branch from b9e29a4 to 5271fdb Compare July 5, 2023 08:03
@sonarcloud
Copy link

sonarcloud bot commented Jul 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
No Duplication information No Duplication information

@julien-deramond julien-deramond merged commit ca97b12 into main Jul 5, 2023
@julien-deramond julien-deramond deleted the main-ic-add-link-to-agree-terms-and-condition-checkbox branch July 5, 2023 08:23
@julien-deramond julien-deramond mentioned this pull request Jul 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@louismaximepiton louismaximepiton louismaximepiton left review comments

@Aniort Aniort Aniort approved these changes

@julien-deramond julien-deramond julien-deramond approved these changes

@MewenLeHo MewenLeHo MewenLeHo approved these changes

Assignees

@isabellechanclou isabellechanclou

Labels
docs Improvements or additions to documentation passed a11y review passed design review v5
Projects
🟣 [Orange-Boosted-Bootstrap] PRs Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs > Component > Forms > Validation: Fix "Agree to terms and conditions" checkbox label
6 participants
@isabellechanclou @Franco-Riccitelli @Aniort @julien-deramond @louismaximepiton @MewenLeHo