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: error when input type is checkbox #8048

Merged
merged 5 commits into from
Oct 25, 2021

Conversation

miguelaeh
Copy link
Contributor

@miguelaeh miguelaeh commented Apr 7, 2021

Description of changes

Extending the SignUp class to implement a custom sign up field like the following:

    this.signUpFields = [
       ....
      {
        label: 'Terms',
        key: 'custom:terms', // Key on the user pool
        required: true,
        displayOrder: 4,
        type: 'checkbox'
      },
     ....

Ends with the following error when clicking signup:

 error: Cannot convert TRUE_VALUE to string

That seems to be because the checkbox value is a true boolean that cannot be converted to a string. With this change instead of leaving JS to make that cast to a string, we force it so in the case of a true boolean "true" as a string will be set.

Issue #, if available

Description of how you validated changes

I validated the changes manually and the issue with the boolean values dissapeared.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@miguelaeh miguelaeh marked this pull request as draft April 8, 2021 08:15
@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #8048 (4a86134) into main (fb28a36) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8048   +/-   ##
=======================================
  Coverage   75.42%   75.42%           
=======================================
  Files         215      215           
  Lines       13548    13548           
  Branches     2673     2673           
=======================================
  Hits        10218    10218           
  Misses       3127     3127           
  Partials      203      203           
Impacted Files Coverage Δ
packages/aws-amplify-react/src/Auth/AuthPiece.tsx 86.95% <100.00%> (ø)
...ges/aws-amplify-react/src/Widget/SelectMFAType.tsx 86.88% <100.00%> (ø)
...ges/aws-amplify-react/src/Widget/TOTPSetupComp.tsx 92.53% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb28a36...4a86134. Read the comment docs.

@miguelaeh miguelaeh marked this pull request as ready for review April 8, 2021 18:22
@siegerts siegerts added the first-time-contributor The contribution is the first for this user in the repo label Jul 2, 2021
@siegerts siegerts requested review from Amplifiyer and a team and removed request for Amplifiyer July 12, 2021 15:50
@evcodes evcodes added this to To Review in Pull Requests via automation Oct 21, 2021
@ashika01 ashika01 self-requested a review October 25, 2021 22:50
@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2021

Codecov Report

Merging #8048 (34b5c1f) into main (7cfe581) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8048   +/-   ##
=======================================
  Coverage   78.02%   78.02%           
=======================================
  Files         250      250           
  Lines       18109    18109           
  Branches     3882     3882           
=======================================
  Hits        14129    14129           
  Misses       3850     3850           
  Partials      130      130           
Impacted Files Coverage Δ
packages/aws-amplify-react/src/Auth/AuthPiece.tsx 86.45% <100.00%> (ø)
...ges/aws-amplify-react/src/Widget/SelectMFAType.tsx 88.40% <100.00%> (ø)
...ges/aws-amplify-react/src/Widget/TOTPSetupComp.tsx 92.10% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cfe581...34b5c1f. Read the comment docs.

package.json Outdated Show resolved Hide resolved
@ashika01 ashika01 changed the title Fix error when input type is checkbox Fix: error when input type is checkbox Oct 25, 2021
@ashika01 ashika01 changed the title Fix: error when input type is checkbox fix: error when input type is checkbox Oct 25, 2021
Copy link
Contributor

@ashika01 ashika01 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I am getting this merged in. :shipit:

@ashika01 ashika01 merged commit 5d01a1d into aws-amplify:main Oct 25, 2021
Pull Requests automation moved this from To Review to Merged Oct 25, 2021
@miguelaeh miguelaeh deleted the booleanError branch December 3, 2021 12:47
@wlee221 wlee221 mentioned this pull request Dec 6, 2021
4 tasks
@github-actions
Copy link

github-actions bot commented Dec 4, 2022

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
first-time-contributor The contribution is the first for this user in the repo
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants