Skip to content
This repository has been archived by the owner on Feb 8, 2023. It is now read-only.

1206-Phone-Number-Error-and-Hint-Text-Post-Circle-PR #1358

Merged
merged 27 commits into from
Jun 23, 2020

Conversation

Dmac26
Copy link
Contributor

@Dmac26 Dmac26 commented Jun 1, 2020

Summary

Addresses Issue # 1165
This PR is an updated version of the original 1165 PR, which needed several styling updates to account for mobile views and to clean up some syntax. I was unable to directly update the original PR as I did not manage my work flow properly and my branch was significantly behind the dev branch.

This pull request is ready to merge when...

  • Feature branch is starts with the issue number
  • Is connected to its original issue via zenhub 👇
  • All tests are passing and meet coverage, linting, and accessibility requirements. And no security vulnerabilities ⚫️(Jenkins)
  • This code has been reviewed by someone other than the original author

Copy link
Contributor

@mtlaney mtlaney left a comment

Choose a reason for hiding this comment

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

Code changes look good and clean. I would revert or omit changes to package-lock.json and package.json, it appears to be trying to upgrade a bunch of stuff we don't need right now, and will cause errors with webdriver trying to build and run the tests in Jenkins.

@Dmac26
Copy link
Contributor Author

Dmac26 commented Jun 10, 2020

@mtlaney - Hey Mike, I went ahead and reverted the .json file.

@briandavidson
Copy link
Member

image

Prior, the user was unable to enter more than the allowed 10 characters. Let's revert back to that functionality, and we should still be able to address the the concerns of the original issue.

Copy link
Member

@briandavidson briandavidson left a comment

Choose a reason for hiding this comment

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

As @mtlaney stated, please revert the changes to package.json and omit package-lock.json.

After that's addressed, and the minor issue with allowing the user to type more than 10 characters, this should be g2g.

server/package.json Outdated Show resolved Hide resolved
@Dmac26
Copy link
Contributor Author

Dmac26 commented Jun 17, 2020

@mtlaney @briandavidson - This one should be good to go too!

Copy link
Member

@briandavidson briandavidson left a comment

Choose a reason for hiding this comment

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

Looks g2g

@briandavidson briandavidson removed their assignment Jun 23, 2020
Copy link
Contributor

@mtlaney mtlaney left a comment

Choose a reason for hiding this comment

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

g2g

@Dmac26 Dmac26 merged commit f6749e1 into dev Jun 23, 2020
@abdul-fs abdul-fs mentioned this pull request Jul 2, 2020
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants