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

Added automatic donate buttons #394

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

DeevanP
Copy link
Contributor

@DeevanP DeevanP commented Jul 8, 2024

For #378

Added donate buttons to each chapter's page, which are now dependant on whether their fundraiser is 'running' (based on Proposed solution: option B). As a result, when a chapter's fundraiser is still 'running', it will display the donation buttons on their page, and should not display the donation buttons when the fundraiser is no longer 'running'.

This feature was also added to the alumni, demo and workplace pages too.

@DeevanP DeevanP requested a review from domdomegg July 8, 2024 18:24
@DeevanP DeevanP self-assigned this Jul 8, 2024
@DeevanP DeevanP linked an issue Jul 8, 2024 that may be closed by this pull request
@DeevanP DeevanP removed the request for review from domdomegg July 8, 2024 18:27
@DeevanP DeevanP requested a review from domdomegg July 8, 2024 18:48
Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

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

Sorry for the delay reviewing this.

Overall this looks good, and I'm excited to not have to manually add all the buttons each year! This will increase load on the backend but I think this is an okay trade-off: we can always come back and review this if my AWS bill shoots up 😅

Two minor pieces of feedback:

  1. I think the logic could be slightly clearer - I've demonstrated this for the alumni page here: be8f86a
  2. When changing the indentation of a lot of code (e.g. 3000 lines - if it was e.g. 30 lines it'd be fine) it can be challenging for a reviewer to figure out the important lines that have changed. In future if making similar changes it'd be helpful to separate out the indentation change to a separate commit, because then the reviewer can see the actual changes separately more easily. Nothing to change for this PR, just hopefully useful feedback for you in future :)

Next steps: Could you apply similar changes to (1) in the other pages? After that I'll get this merged! :)

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.

Automatic donate buttons on chapter pages
2 participants