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

feat(donate):add donation modal and certification message #37822

Merged
merged 37 commits into from Dec 2, 2019

Conversation

ahmaxed
Copy link
Member

@ahmaxed ahmaxed commented Nov 24, 2019

  • I have read freeCodeCamp's contribution guidelines.
  • My pull request has a descriptive title (not a vague title like Update index.md)
  • My pull request targets the master branch of freeCodeCamp.
  • None of my changes are plagiarized from another source without proper attribution.
  • All the files I changed are in the same world language (for example: only English changes, or only Chinese changes, etc.)
  • My changes do not use shortened URLs or affiliate links.

Closes #XXXXX

@gitpod-io
Copy link

gitpod-io bot commented Nov 24, 2019

@ahmaxed ahmaxed changed the title feat(donate):add donation initial feat(donate):add donation modal and flash Nov 25, 2019
@ahmaxed ahmaxed force-pushed the feat/donation-modal branch 3 times, most recently from 2f3b5a1 to 0b57f47 Compare November 27, 2019 17:17
@ahmaxed ahmaxed marked this pull request as ready for review November 27, 2019 20:41
@ahmaxed ahmaxed requested a review from a team November 27, 2019 20:41
@ahmaxed
Copy link
Member Author

ahmaxed commented Nov 27, 2019

This pr will display a donation message on a certificate if the username of the certificate matches the username of the signed in user.

The message will disappear for the session if user clicks on the message link and get redirected to donate page.

Additionally a modal will be displayed after the last challenge of a block has been submitted.

if either of the donation requests has been shown during a session, the other one will not be shown.

Please let me know if the logic is in place so I could proceed with tests.

@ahmaxed ahmaxed changed the title feat(donate):add donation modal and flash feat(donate):add donation modal and certification message Nov 28, 2019
@ahmaxed
Copy link
Member Author

ahmaxed commented Nov 28, 2019

@ojeytonwilliams, could you take a quick look?

@raisedadead
Copy link
Member

raisedadead commented Nov 28, 2019

Please provide detailed context about when this modal is to be shown for QA in the OP.

@ahmaxed
Copy link
Member Author

ahmaxed commented Nov 28, 2019

@raisedadead, the modal will be shown after the last challenge on a block has been completed.
it will check if the user has donated, if any other form of donation has been requested (certificate message) in that session.

an easy way to test it would be the data viz certificates. the donation should pop up after the last challenge has been completed.

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

The UX is great, but I'm not sure if it's always behaving exactly as it's supposed to and there's potentially an issue for slow connections:

client/src/client-only-routes/ShowCertification.js Outdated Show resolved Hide resolved
client/src/client-only-routes/ShowCertification.js Outdated Show resolved Hide resolved
@ahmaxed
Copy link
Member Author

ahmaxed commented Dec 1, 2019

@ojeytonwilliams could you take another look?

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

I noticed I'd named an action badly, so I added some suggestions to fix that. I didn't change donationRequested to preventDonationRequests, because I was worried I'd miss one. I can add that as a separate commit.

As far as functionality goes, everything looks good. The following is just nitpicking about names:

client/src/assets/icons/Heart.js Outdated Show resolved Hide resolved
client/src/assets/icons/Heart.js Outdated Show resolved Hide resolved
client/src/assets/icons/Heart.js Outdated Show resolved Hide resolved
client/src/assets/icons/Heart.js Outdated Show resolved Hide resolved
client/src/components/Donation/components/DonationModal.js Outdated Show resolved Hide resolved
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
ahmaxed and others added 19 commits December 2, 2019 14:17
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
…est.js

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
…est.js

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
…est.js

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
…est.js

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
…est.js

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

Eslint's complaining about the constructor:

ahmaxed and others added 4 commits December 2, 2019 14:52
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
@ahmaxed ahmaxed merged commit a9bbcda into freeCodeCamp:master Dec 2, 2019
@ahmaxed ahmaxed deleted the feat/donation-modal branch December 2, 2019 12:48
abbathaw pushed a commit to abbathaw/freeCodeCamp that referenced this pull request Jul 24, 2020
…mp#37822)

Co-Authored-By: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants