FreeUKGen / Systemwide

repository for issues that affect all systems within the Free UK Gen portfolio
0 stars 0 forks source link

Project Donations pages #215

Open DeniseColbert opened 3 years ago

DeniseColbert commented 3 years ago

We have this donation widget/tool: https://freeukgen.github.io/FreeUKGenealogyWP/inner_donation.html

It can be served on any/all of our websites, making our goal of keeping donors on the site they're on much easier to achieve.

So, plan:

E8 U6 S7 C5: P26

See also closed story on FreeREG

Captainkirkdawson commented 3 years ago

We need to be careful with the design of all such pages that we are not opening ourselves to hacking and misdirection of donations. Hackers are fair more resourceful than we are.

TazHinkle commented 3 years ago

The tool as it exists now is not any more vulnerable than something comparable made in WordPress (though there is a certain elegance of having everything hosted in the same place that is missing). Someone without access to either upload content to our github (users have to be added) or with access to the WP tools should have no way to change the links that users are clicking on. All sensitive information is handled after they have used the tool to select a third party so those transactions are protected by kindlink/paypal.

The most common redirection scams have to do with emailing people links to similar looking pages that have their updated, malicious links. There's no real way to stop people from doing that (so far as I know), but as for changing the links on our actual website, they would have to have been given permission to edit by your organization on either WP or github.

AlOneill commented 3 years ago

From an Accessibility standpoint, although the individual radio buttons have labels, the group as a whole does not and the 'heading' of Choose currency is neither a heading nor a label.

There is no visible focus on the 'Next' button — hover is marked by underlining.

On selecting Next, the buttons revealed have good visible focus, except for 'Previous' which has none. The visible focus for 'Donate by Card via Kindlink' is rather weak (browser default styling only).

TazHinkle commented 3 years ago

Good points about Accessibility. I believe all of these can be resolved fairly painlessly. I can spend some time working on these in the next little while, however it might be most efficient to have your style people make rulings on preferred focus/coloring for previous/outline width's etc. The more detailed and exact the better.

TazHinkle commented 3 years ago

I have been working on this. I have made some changes to the active version (mostly the visibility of the Previous button). I have a version where the appearance for the 'Donate by Card via Kindlink' is adjusted that could be put into play if you feel it's better. I'm posting an image below showing, left to right: tool as it was (on WordPress) before I started adjusting yesterday, as it is uploaded right now, and the version that only exists on my computer: evolution of donate tool.png

TazHinkle commented 3 years ago

The tool is now available with several visibility (and uniformity) revisions. I will need further guidance on other changes you would like implemented.

AlOneill commented 3 years ago

Thank you for making "Select currency" a heading — h3 might not turn out to be the correct level, depending on how this will be used. However, the radio buttons should be grouped, typically in a fieldset and "Select Currency" would make a good legend.

There is still no visible focus on either the "Next" or "Previous" buttons. The only indicator on hover of the "Donate by Card via kindlink" button is that the cursor changes to a pointer.

@PatReynolds @DeniseColbert What do you think of the hover states — are the changes enough?

TazHinkle commented 3 years ago

Is it just that you want the appearance of Next and Previous to change as you're clicking- for the length of time it takes the next part to load, or is there a place where you want them to remain focused when the resulting content loads?

This may be helpful for one of the decisions: For the currency selection screen I just did another side by side (by side) comparison (in Paint) of what it looks like in a fieldset with a legend, how it looks now, and the Figma version done by your 48in48 designer.

image.png

AlOneill commented 3 years ago

Having visible focus is important for a sighted keyboard-only user: they navigate by using the tab key and need to be able to see which element, which link or button (or form input), has the :focus state and so can be selected, using enter or space. (I think you were describing the :active state.)

I like the use of words for the currencies in the Figma version — we would need to be consistent with singular or plural forms and also with casing. (FWIW, I think singular works well.)

How does it look if you make the legend invisible, using class="accessibility" (which all projects have) and maybe make the fieldset border invisible, by making it white, say? The accessibility class keeps an element 'visible' to screen-reader software. We would then make the heading 'invisible' to a screen-reader user with Aria, so the user does not hear 'select currency' twice.

TazHinkle commented 3 years ago

Thank you for the clarifications, it's helped a lot.

I've acted on the fieldset/legend and focus items. Let me know how those are working out.

I have waited to change the currency names because I was instructed to change them to the three letter variant by another member of your team. I wanted to give you guys time to discuss, but when there's a final decision, it's an amazingly easy edit.

I have also waited on adjusting the hover features. The 'Donate via Kindlink' button is a button drawn around an image- care of Kindlink (I understand that, like paypal, that is their preferred method of linking)- so underline for text is not available. There are other hover options that can be enabled. I think it's worth pointing out that hover is lost on any touchscreen device. However, if there is a specific other hover effect that is desired, we can (probably) do that.

AlOneill commented 3 years ago

Ah! I was not thinking clearly when I described the "hiding" process. Second go …

As your widget does not have access to our CSS (yes?) you will need the details for a cross-browser accessibility class —

.accessibility { 
border: 0 !important;
clip: rect(0 0 0 0) !important;
height: 1px !important;
margin: -1px !important;
overflow: hidden !important;
padding: 0 !important;
position: absolute !important;
width: 1px !important;
}

Please then remove the hidden attribute and aria-hidden="true" from the legend element. They would render the legend invisible to a screen-reader (!) which is the opposite of what we need here. We want the screen-reader to 'see' the legend but it should be out of sight.

The first heading should be an h1 element as the widget is, in effect a page, yes? And subsequent heading levels should be in proper order (hierarchy). However there are currently 3 of h3 and 4 of h4. So I think we should create an h1.accessibility at the top of the page — the content should let a non-sighted user know what has happened — they need to know where they are. Maybe 'Set up your donation' or something similar? Then promote the remaining headings to h2 and h3 respectively. Does that make sense to you?

The Next and Previous buttons are fixed — thank you. However, there is an invisible element that is brought into focus (invisibly!) between the Kindlink button and the Previous button — I think it is because the button is nested inside an a which gives 2 focusable elements. We need just one (somehow)!

[edited to fix formatting!]

TazHinkle commented 3 years ago

I may have accomplished the things you've asked for.

AlOneill commented 3 years ago

You have! Thank you.

The Wave tool (from the WebAim site) is reporting no errors and just two alerts: it is suggesting we add a fieldset and legend for the two Gift Aid choices, which seems reasonable, and that we consider using Aria role="region" — I'm less convinced about that but I'm no Aria expert and you might disagree (we are aiming to comply with WCAG 2.0).

The look of the headings, the h2 and h3 elements, now that they are hierarchical, is not the best and could do with some tweaks to restore the visual balance of before. With those fixed, I think we will be done, pending the verdict on the currency labels!

TazHinkle commented 3 years ago

For the Aria role="region"- I think it would be ok if we skipped doing this, but it might be a nice thing... I looked into it a little bit. MDN says that the same function can be achieved (and be better semantically) with section tags. Were you thinking of landmarking the currency selector radios? The whole widget?

I'm definitely still learning about the best ways to do accessibility.

TazHinkle commented 3 years ago

I've now adjusted down the size of the h2 and put the whole thing in a section tag (we'll see if that has the effect that was desired... I have noticed no adverse effects for adding it). The h3 currency labels look nice and super readable to me so I left them alone (for now). At this point, I'm going to wait to do any more adjustments until I get more instructions.

Thanks AlOnneill and all for your feedback.

AlOneill commented 3 years ago

Looking good!

There are a couple of things that bother me:

  1. Having selected a currency, the next has view has the 'Donate by Card' button pre-focussed, so it was not obvious to me that the Kindlink button at the bottom goes with it. Selecting Donate by Card button, with nothing changing, stalled me for a spell until I selected some other buttons and realised what was happening (sigh!).
  2. How does a screen-reader user know what to do next, after selecting Donate by Card? I'm wondering if another radio-group would work better here, so that on pressing tab, the focus leaves the group and moves into the additional content for that choice. Can you foresee any problems with that approach?

At some point I'll attempt to hear the widget with VoiceOver — I might then be able to offer something more useful.

I'm not a great user of widgets and have never made one, so I'm gonna ask a dumb (!) question — is the current layout and/or markup a standard pattern? Are users, particularly blind ones, likely to recognise it?

richpomfret commented 3 years ago

@Vino-S to investigate any possible security risk here.

DeniseColbert commented 3 years ago

@PatReynolds @DeniseColbert and @AlOneill to discuss currency display, and point 1.

@TazHinkle to test if widget allows external links to open in a new tab

Taz and Denise discussed potential for different versions of the tool to be offered dependent on screen-reader/keyboard enter-strike. Taz thinks both are possible (and others potentially?) but keyboard strike opens up more possibility for premature navigation e.g. people who are just clicking around the widget, seeing the options. I think this might be an acceptable situation. What do others think?

TazHinkle commented 3 years ago

Links that would navigate offsite now open in new tabs.

PatReynolds commented 3 years ago

@Vino-S to look at security risks, and team to look at other issues in Documenation meeting

DeniseColbert commented 3 years ago

Currency display Decision: Full words, include symbols

UK Pound (£) Australian Dollar ($) Canadian Dollar ($) NZ Dollar ($) US Dollar ($) Euro (€)

Re. Point 1 above @DeniseColbert to find model widget and place link in story, to revisit next meeting

Place "opens in a new tab" text above button/link

image

DeniseColbert commented 3 years ago

@Vino-S please could you send by email or slack, the results of your review (as above) for @TazHinkle

DeniseColbert commented 3 years ago

Website with a widget (for pre-focus issue) https://teamtrees.org/

DeniseColbert commented 2 years ago

Add: "Choose how to donate below" (pre-focus) Nothing in lower section, to appear upon selection

Suggest PayPal link in top button rather than PayPal button in lower section (same for KL) - @DeniseColbert to discuss with @TazHinkle

TazHinkle commented 2 years ago

I haven't heard from anyone for a bit, but I had a little time today. Not sure if you decided on changing the order of the buttons so I left those alone. The rest of it I have tried to change to your stated preferences. I've committed the changes and it's ready for you to look over.

PatReynolds commented 2 years ago

@DeniseColbert to take a look and let @TazHinkle know the order of buttons.