SL-346: add accessibility improvements for EAA compliance#320
SL-346: add accessibility improvements for EAA compliance#320GytisZum wants to merge 1 commit intoSL-358/payment-fieldfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the application's accessibility to meet EAA compliance standards. The changes focus on making the user interface more navigable and understandable for users relying on assistive technologies, by providing clearer semantic structures and interactive feedback across key components like forms, modals, and payment processes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of accessibility improvements across various templates and JavaScript files. Key changes include enhancing form element labeling, providing descriptive aria-label and alt attributes, improving modal dialog semantics with ARIA roles, and ensuring interactive elements like buttons are semantically correct. These changes significantly improve the user experience for individuals using assistive technologies, aligning the application with better accessibility standards.
| function closeModal($modal) { | ||
| $modal.removeClass('open'); | ||
| var triggerButton = $modal.data('triggerButton'); | ||
| if (triggerButton) { | ||
| triggerButton.focus(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The closeModal function correctly handles closing the modal and returning focus to the trigger button. This is a crucial accessibility pattern for modal dialogs, ensuring keyboard users can easily navigate back to their previous context.
| $(document).on('keydown', function (event) { | ||
| if (event.key === 'Escape') { | ||
| var $openModal = $('.modal.open'); | ||
| if ($openModal.length) { | ||
| closeModal($openModal); | ||
| event.preventDefault(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Implementing the 'Escape' key to close the modal is an excellent accessibility feature, providing an intuitive way for keyboard users to dismiss the dialog.
| // NOTE: opening modal | ||
| $('#' + $(this).data('target')).addClass('open'); | ||
| $modal.addClass('open'); | ||
| $modal.find('.js-log-modal-close').focus(); |
There was a problem hiding this comment.
Setting focus to the close button when the modal opens is a good practice for accessibility, as it immediately places the user's focus within the modal and provides an obvious way to exit.
| type="button" | ||
| onClick={() => setShowApiPassword(!showApiPassword)} | ||
| className="sp-absolute sp-right-3 sp-top-1/2 sp--translate-y-1/2 sp-text-muted-foreground hover:sp-text-foreground sp-transition-colors" | ||
| className="sp-absolute sp-right-3 sp-top-1/2 sp--translate-y-1/2 sp-text-muted-foreground hover:sp-text-foreground sp-transition-colors sp-p-1 sp-min-w-[24px] sp-min-h-[24px] sp-flex sp-items-center sp-justify-center" |
There was a problem hiding this comment.
Adding sp-p-1 sp-min-w-[24px] sp-min-h-[24px] sp-flex sp-items-center sp-justify-center to the password toggle button improves its clickable area and visual consistency, which enhances usability and accessibility for users with motor impairments or those using touch interfaces.
|
|
||
| <div className="sp-flex sp-justify-end"> | ||
| <Button className="sp-min-w-[120px]" onClick={saveCredentials} disabled={saving}> | ||
| <Button className="sp-min-w-[120px]" onClick={saveCredentials} disabled={saving} aria-label={saving ? t('saving') : undefined}> |
There was a problem hiding this comment.
Adding a dynamic aria-label to the save button that indicates its 'saving' state provides crucial feedback for screen reader users, improving the accessibility of the form submission process.
| <button type="submit" class="btn btn-default button button-medium"> | ||
| <span>{l s='Pay' mod='saferpayofficial'}<i class="icon-chevron-right right"></i></span> | ||
| </a> | ||
| </button> |
There was a problem hiding this comment.
Changing the <a> tag to a <button type="submit"> is a crucial semantic and accessibility improvement. Buttons are designed for actions like form submission, providing correct semantics and expected behavior for assistive technologies.
| <label> | ||
| <input type="radio" name="saved_card_{$paymentMethod|escape:'htmlall':'UTF-8'}" value="{$savedCard['id_saferpay_card_alias']|escape:'htmlall':'UTF-8'}" | ||
| {if $selected }checked="checked" {$selected = 0}{/if} | ||
| > | ||
| <span>{$savedCard['card_number']|escape:'htmlall':'UTF-8'}</span> | ||
| </label> |
There was a problem hiding this comment.
Wrapping each radio input and its associated text within a <label> tag is the correct and most accessible way to associate labels with radio buttons. This makes the entire label clickable and improves usability for all users.
| <label> | ||
| <input type="radio" name="saved_card_{$paymentMethod|escape:'htmlall':'UTF-8'}" value="0" | ||
| {if $selected }checked="checked"{/if} | ||
| > | ||
| <span>{l s='Use new card and save it' mod='saferpayofficial'}</span> | ||
| </label> |
There was a problem hiding this comment.
Wrapping each radio input and its associated text within a <label> tag is the correct and most accessible way to associate labels with radio buttons. This makes the entire label clickable and improves usability for all users.
| <label> | ||
| <input type="radio" name="saved_card_{$paymentMethod|escape:'htmlall':'UTF-8'}" value="-1"> | ||
| <span>{l s='Use new card once' mod='saferpayofficial'}</span> | ||
| </label> | ||
| </div> |
There was a problem hiding this comment.
Wrapping each radio input and its associated text within a <label> tag is the correct and most accessible way to associate labels with radio buttons. This makes the entire label clickable and improves usability for all users.
|
|
||
| <div> | ||
| <iframe id="test-frame" src=''></iframe> | ||
| <iframe id="saferpay-payment-frame" src='' title="{l s='Saferpay payment form' mod='saferpayofficial'}"></iframe> |
There was a problem hiding this comment.
Changing the iframe id to saferpay-payment-frame and adding a descriptive title attribute are good practices. The title attribute is particularly important for accessibility, providing context for screen reader users about the iframe's content.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
1 participant
Self-Checks
JIRA task link
Summary
QA Checklist Labels
QA Checklist
Additional Context
Frontend Changes