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

DON-1066: Link to regular giving agreement from card payment #1800

Merged
merged 2 commits into from
Dec 16, 2024
Merged

DON-1066: Link to regular giving agreement from card payment #1800

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aae2377
DON-1066: Link to regular giving agreement from card payment
bdsl Dec 11, 2024
d28cc01
DON-1066: Remove link from donation to campaign.
bdsl Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/app/donation.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ export const maximumDonationAmountForFundedDonation = 200_000;
* * after the donation is fully processed and webhook returned (e.g. `matchedAmount`).
*/
export interface Donation {
/**
* The regular giving agreement relating to this donation, if any. Optional property because production
* matchbot doesn't yet send it. Not a full representation of the mandate, just enough to be able to render
* a link to it etc.
*/
mandate?: {
'uuid': string,
'activeFrom': string,
},

autoConfirmFromCashBalance?: boolean;

/**
Expand Down
9 changes: 7 additions & 2 deletions src/app/my-donations/my-donations.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,13 @@
>
<div class="donation">
<div>
<p>{{this.displayMethodType(donation)}}</p>
<h3><a href="/campaign/{{donation.projectId}}">{{ donation.charityName }}</a></h3>
<p>{{this.displayMethodType(donation)}}
@if (donation.mandate) {
<br />
<a href="{{myRegularGivingPath + '/' + donation.mandate.uuid}}">Regular giving since {{donation.mandate.activeFrom | date: 'mediumDate'}}</a>
}
</p>
<h3><a class="charityName" href="/campaign/{{donation.projectId}}">{{ donation.charityName }}</a></h3>
</div>
<ul>
<li>Amount: {{ donation.donationAmount | exactCurrency:donation.currencyCode }}</li>
Expand Down
5 changes: 4 additions & 1 deletion src/app/my-donations/my-donations.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
main > div {
color: white;
background-color: $colour-primary;
}

p:not(biggive-container-card *) {
// biggive-container-cards have white backgrounds so white links in there would be invisible.
a:link, a:visited, a:hover {
color: white;
}
Expand All @@ -24,7 +27,7 @@ div.donation {
height: 100%;
justify-content: space-between;
flex-direction: column;
a:link, a:visited, a:hover {
a.charityName:link, a.charityName:visited, a.charityName:hover {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the mix of link styles here, and the card not all being linked like the home highlight cards. Is there a way donors would figure out that a bold charity name is a mandate link based on other styling around the app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The charity name isn't a link to the mandate, it's a link to the campaign. The link to the mandate is blue and not bold.

I'm also not sure about mixing the styles though. The campaign link style is unchanged. I can't remember for sure why we didn't make that one blue but I guess we thought it was not very important and having it blue might make the page too busy.

The mandate links are probably more vital for people to be able to find easily.

Copy link
Contributor

@NoelLH NoelLH Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Right, sorry for typo. I think I did see what it linked to but still feel it's better to either re-style the campaign link somehow or not link at all.

I agree the mandate one is the priority and is good as-is. I don't think that precludes the rest of the card effectively becoming a big campaign link if we can make that work, since that mostly mirrors a design pattern we have already used on the home page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NoelLH do you have a view on a way forward for this? I looked on the comment on DON-983 to see if we discussed the links being black rather than blue. There are a lot of comments there so I may have missed it, but I can't see that we did.

I think having the entire card linked probably doesn't make sense unless it was a link to the donation, which we aren't currently doing. For now we link the campaign and the mandate. I think we may well want to add a link to the donation as a its own page (which may look exactly the same as our current donation thank you page) in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(wrote my 11:18 message before I saw your 11:11 message)

Copy link

Choose a reason for hiding this comment

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

Thanks @bdsl from a design perspective, there is a lot of white space on the right hand side that it would be good to use / remove if possible, maybe through more padding on the left

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DomTBG The width is going to be quite dependant on the number of donations you have and the width of your screen, and if we adjust it we'll need to test with a range of screen sizes again so I don't know if we need to get into that now - it doesn't seem like an issue that comes from adding the link to the regular giving mandate.

Do you have a view about the link styling for the link to the mandate and the link to the charity?

Copy link

Choose a reason for hiding this comment

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

OK @bdsl let's leave the white space question for now.
I don't know that we need to link to the campaign for any donations - it's not like we link to the campaign in the donation receipt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DomTBG No, but the donation receipt (template here ) does link to the charity's own website, as well as giving there postal address, phone number etc.

I reviewed the Jira comments on DON-983 when we introduced this page, sadly there doesn't seem to be any record of discussion about the links to campaign pages. I think I just included them in my initial version of this page and no-one objected.

But I've pushed commit d28cc01 here to remove the link to the campaign so for now if you have a regular giving donation and an ad-hoc donation they will look like this:

image

I do think at some point we're likely to want to add in a link to a full page of details about each donation, we may have to revisit this design then.

Copy link

Choose a reason for hiding this comment

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

we are considering removing all of that info on the donation receipt though @bdsl

color: inherit !important;
text-decoration: none;
}
Expand Down
2 changes: 2 additions & 0 deletions src/app/my-donations/my-donations.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {AsyncPipe, DatePipe} from "@angular/common";
import {ExactCurrencyPipe} from "../exact-currency.pipe";
import {MatProgressSpinner} from "@angular/material/progress-spinner";
import {allChildComponentImports} from "../../allChildComponentImports";
import {myRegularGivingPath} from "../app-routing";

@Component({
selector: 'app-my-donations',
Expand All @@ -23,6 +24,7 @@ import {allChildComponentImports} from "../../allChildComponentImports";
export class MyDonationsComponent implements OnInit{
protected donations: EnrichedDonation[];
protected atLeastOneLargeRecentDonation: boolean;
protected readonly myRegularGivingPath = myRegularGivingPath;

constructor(
private pageMeta: PageMetaService,
Expand Down
6 changes: 1 addition & 5 deletions src/app/regular-giving/regular-giving.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,11 @@ export class RegularGivingComponent implements OnInit {
this.donor = donor;

if ( !this.campaign.isRegularGiving ) {
throw new Error("Campaign " + this.campaign.id + " is not a regular giving campaign");
console.error("Campaign " + this.campaign.id + " is not a regular giving campaign");
}

this.campaign = this.route.snapshot.data.campaign;

if ( !this.campaign.isRegularGiving ) {
throw new Error("Campaign " + this.campaign.id + " is not a regular giving campaign");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

duplicated the previous throw.

this.mandateForm = this.formBuilder.group({
donationAmount: ['', [
requiredNotBlankValidator,
Expand Down
Loading