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

CheckboxInput does not display any InvalidHint to the user #2400

Closed
4 tasks done
r-bartolomei opened this issue Jul 14, 2023 · 6 comments · Fixed by #2423
Closed
4 tasks done

CheckboxInput does not display any InvalidHint to the user #2400

r-bartolomei opened this issue Jul 14, 2023 · 6 comments · Fixed by #2423
Labels
bug This points to a verified bug in the code needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue

Comments

@r-bartolomei
Copy link

Checklist

  • I have looked into the Readme and Examples, and have not found a suitable solution or answer.
  • I have searched the issues and have not found a suitable solution or answer.
  • I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • I agree to the terms within the Auth0 Code of Conduct.

Description

If you use additionalSignUpFields option with a type: 'checkbox' that has a validator function (https://auth0.com/docs/libraries/lock/lock-configuration#checkbox-field), the src/ui/input/checkbox_input.jsx is not wrapped within an InputWrap component but within a simple div and as a result, the invalidHint will never be displayed when you press on the Sign Up button.

Reproduction

with the below code:

new Auth0Lock(AUTH0_CLIENT_ID, AUTH0_DOMAIN, {
    container: LOCK_CONTAINER_ID,
    allowLogin: false,
    allowSignUp: true,
    initialScreen: 'signUp',
    mustAcceptTerms: true,
    additionalSignUpFields: [
        {
            type: 'checkbox',
            name: 'newsletter',
            prefill: 'false',
            placeholder: 'I hereby agree that I want to receive marketing emails from your company',
            validator: (value) => ({
                valid: value === 'true',
                hint: 'This is a mandatory field',
            }),
        },
    ],
});

because of valid: value === 'true', the field becomes mandatory (the user needs to check it) before being able to finish the Sign Up.
But because of the missing InputWrap within the CheckboxInput (src/ui/input/checkbox_input.jsx), no error message will be shown to the user if the checkbox is not checked and the user will not know why he can't go through with the sign up process.

As a comparison, src/ui/input/text_input.jsx and src/ui/input/select_input.jsx have the elements wrapped within the InputWrap and those components are displaying the invalidHint correctly.

Additional context

No response

Lock version

"auth0-lock": "12.0.2"

Which browsers have you tested in?

Chrome

@r-bartolomei r-bartolomei added the bug This points to a verified bug in the code label Jul 14, 2023
@frederikprijck frederikprijck added the needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue label Jul 17, 2023
@frederikprijck
Copy link
Member

Thanks for reaching out. I will look into this and get back to you if I need more info (or have a solution).

@ewanharris
Copy link
Contributor

Hey @r-bartolomei, I just put up a PR that would solve this issue over at #2423. Would love to get your feedback on whether the current styling (checkbox text turned to red) is expected for you if there's an alternative.

@r-bartolomei
Copy link
Author

Hey @ewanharris. If the red border is not possible then the checkbox text on default colour I think might look better because it would align with the rest of mandatory fields (if any) as they would not have the text content on red.

image

@stevehobbsdev stevehobbsdev linked a pull request Aug 22, 2023 that will close this issue
7 tasks
@ewanharris
Copy link
Contributor

Sorry for the delay @r-bartolomei, we've chatted about this and after a little investigation I think we might do the following:

  1. If invalidHint is provided then we'll only make that text red
  2. If invalidHint is not provided, then we'll make the placeholder content red as without that there is no other way to flag the required field

github-merge-queue bot pushed a commit that referenced this issue Sep 14, 2023
### Changes

Wrap the `CheckBoxInput` in `InputWrapper` to allow for providing
feedback as to what is wrong when a user clicks submit without the
checkbox being selected. Below is an example of what this looks like,
rather than highlight using a border like other fields we change the
text to red.

**This image is outdated see below**
<img width="301" alt="Screenshot 2023-08-04 at 12 03 18"
src="https://github.com/auth0/lock/assets/8705251/cb346e0a-5d3e-4d24-87e8-b037b99dd34b">

I did also see what it looked like without the red text, which is a
little less obvious.

<img width="300" alt="Screenshot 2023-08-04 at 11 59 11"
src="https://github.com/auth0/lock/assets/8705251/688ee97f-cd17-4375-80bb-b00d252529cb">

This is if we add a border around the entire element like the other
components
![Screenshot 2023-08-04 at 11 05
07](https://github.com/auth0/lock/assets/8705251/a3eea2dc-c8ee-4e90-b5b9-f7fa68e3d143)

I did also try a border/outline around the checkbox element but it
looked bad due to inability to set the border radiu


Happy to change this up if there's a general preference that looks
better.


Current status:

Invalid hint text provided
![Invalid hint text is red and aligned with checkbox
text](https://github.com/auth0/lock/assets/8705251/9ec1f0d1-9d60-46fd-b38b-9165dbc9ae71)

No invalid hint text provided
![Checkbox text is
red](https://github.com/auth0/lock/assets/8705251/c9452810-11f3-43e3-85e6-bc4298f5d904)


### References

#2400

### Testing

<!--
Please describe how this can be tested by reviewers. Be specific about
anything not tested and reasons why. If this library has unit and/or
integration testing, tests should be added for new functionality and
existing tests should complete without errors.
-->

* [x] This change adds unit test coverage
* [ ] This change adds integration test coverage
* [ ] This change has been tested on the latest version of the
platform/language

### Checklist

* [x] I have read the [Auth0 general contribution
guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md)
* [x] I have read the [Auth0 Code of
Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)
* [x] All code quality tools/guidelines have been run/followed
* [x] All relevant assets have been compiled
@ewanharris
Copy link
Contributor

Hey @r-bartolomei, this change shipped today in version 12.2.0, thanks for the request!

@r-bartolomei
Copy link
Author

@ewanharris Thank you for fixing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This points to a verified bug in the code needs investigation An issue that has more questions to answer or otherwise needs work to fully understand the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants