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

Strip amount $ #1065

Merged
merged 4 commits into from
Oct 25, 2023
Merged

Strip amount $ #1065

merged 4 commits into from
Oct 25, 2023

Conversation

wrandall22
Copy link
Contributor

@wrandall22 wrandall22 commented Oct 25, 2023 •
edited
Loading

This PR strips out $ from the gift amount in the product config form.

@wrandall22 wrandall22 requested a review from dr-bizz October 25, 2023 17:54
@wrandall22 wrandall22 changed the title Strip amount $ and , Strip amount $ Oct 25, 2023
dr-bizz
dr-bizz approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

JS looks good. I don't see it on stage to test, but I'll approve it for now and test locally

@wrandall22
Copy link
Contributor Author

Can only test on the parallel stage environment at the moment. It is unfortunately still not right due to how parseFloat handles commas.

@dr-bizz
Copy link
Contributor

dr-bizz commented Oct 25, 2023

I see

@wrandall22
Copy link
Contributor Author

The mix of this and the other branch (better error message) are available to test on https://give-stage2-next.cru.org/

@wrandall22 wrandall22 added the On Staging Will be merged to the staging branch by Github Actions label Oct 25, 2023
@wrandall22 wrandall22 merged commit 224ccc5 into master Oct 25, 2023
5 checks passed
@wrandall22 wrandall22 deleted the strip-dollar-sign branch October 25, 2023 20:22
@wrandall22 wrandall22 mentioned this pull request Oct 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-bizz dr-bizz dr-bizz approved these changes

@hlbraddock hlbraddock hlbraddock approved these changes

Assignees
No one assigned
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wrandall22 @dr-bizz @hlbraddock