-
Notifications
You must be signed in to change notification settings - Fork 153
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
11177 donate help page notice #11197
11177 donate help page notice #11197
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks @danielfmiranda! I just have a question regarding rendering the body
field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Daniel, great work!
It's looking great! I have a few additional recommendations if you have the time to execute them. However, they are detailed and I understand if they are out of scope!
For tablet (after the break point of 768) I recommend pushing the icon above into the stacked position like on mobile. As with mobile, I also recommend left aligning the icon to be more cohesive with the page structure. This will also allow you to balance the icons size as it currently seems small in comparison to the copy content.
Current with change recommendations:
Mockup of changes:
For mobile, the icon size can be reduced and the icon can be shifted to left-aligned to be more cohesive with the rest of the page rules / structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Hi @tessheinricks! Thanks very much for the feedback. I have updated the template accordingly, and the page is available for QA testing here: https://donate-foundation-s-11177-dona-fjocje.mofostaging.net/en/help/ Can you please take another look whenever you have a free moment and let me know what you think? Thanks in advance! 🙌 |
network-api/networkapi/templates/donate/fragments/help_page_notice.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @danielfmiranda! Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @danielfmiranda ! I just have one question regarding the "donation" url we are including in the notice.
network-api/networkapi/templates/donate/fragments/help_page_notice.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨
Description
Link to sample test page: https://donate-foundation-s-11177-dona-fjocje.mofostaging.net/en/help/
Related PRs/issues: #11177
This PR introduces "delayed response notice" at the top of the donate help page by adding the following to the code base:
DonateHelpPage
titledshow_notice
. The delayed response notice will render based on whether or not this field is checked.Screenshots
show_notice
is checked:show_notice is unchecked:
Steps to test
show_notice