Skip to content

Accessibility: add some aria-hidden, labels...#936

Merged
tblivet merged 4 commits intoPrestaShop:2.xfrom
yannicka:feature/a11y
Feb 20, 2026
Merged

Accessibility: add some aria-hidden, labels...#936
tblivet merged 4 commits intoPrestaShop:2.xfrom
yannicka:feature/a11y

Conversation

@yannicka
Copy link
Contributor

@yannicka yannicka commented Feb 18, 2026

Questions Answers
Description? Improve accessibility. Add label, add aria-hidden, add aria-label, etc.
Type? improvement (on accessibility)
BC breaks? no
Deprecations? no
Fixed ticket? N/A
Sponsor company N/A
How to test? N/A

@yannicka yannicka marked this pull request as draft February 18, 2026 14:13
@github-project-automation github-project-automation bot moved this to Ready for review in PR Dashboard Feb 18, 2026
@yannicka yannicka changed the title WIP: Accessibility: improve accessibility Accessibility: improve accessibility Feb 18, 2026
@ps-jarvis
Copy link

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

  • Modules.Emailsubscription.Shop
    • Subscribe to our newsletter
  • Shop.Theme.Catalog
    • Search products...
    • Clear search
  • Shop.Theme.Global
    • Product availability:
    • Open zoomed product image gallery

(Note: this is an automated message, but answering it will reach a real human)

@yannicka
Copy link
Contributor Author

yannicka commented Feb 18, 2026

I also have a few questions:

  1. Is tabindex="0" a good practice on alerts? For example:

    <div class="alert alert-danger alert-dismissible" role="alert" tabindex="0">

  2. Are aria-label useful on contact info? For example:

    aria-label="{l s='Send us an email to: %email%' sprintf=['%email%' => $contact_infos.email] d='Shop.Theme.Global'}">

@yannicka yannicka changed the title Accessibility: improve accessibility Accessibility: improve accessibility (aria-hidden, add label...) Feb 18, 2026
@yannicka yannicka changed the title Accessibility: improve accessibility (aria-hidden, add label...) Accessibility: add some aria-hidden add labels...) Feb 18, 2026
@yannicka yannicka changed the title Accessibility: add some aria-hidden add labels...) Accessibility: add some aria-hidden, labels...) Feb 18, 2026
@yannicka yannicka changed the title Accessibility: add some aria-hidden, labels...) Accessibility: add some aria-hidden, labels... Feb 18, 2026
@yannicka yannicka marked this pull request as ready for review February 18, 2026 16:15
Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Hi @yannicka, thanks for the PR! Here’s some feedback.

@tblivet
Copy link
Contributor

tblivet commented Feb 19, 2026

I also have a few questions:

  1. Is tabindex="0" a good practice on alerts? For example:
    <div class="alert alert-danger alert-dismissible" role="alert" tabindex="0">
  2. Are aria-label useful on contact info? For example:
    aria-label="{l s='Send us an email to: %email%' sprintf=['%email%' => $contact_infos.email] d='Shop.Theme.Global'}">

Hi @yannicka,

  1. I believe tabindex="0" is appropriate in this case because the notification template may display errors or important messages. Without it, the alert wouldn’t be focusable via the Tab key, which could make it harder for keyboard and screen reader users to access.
  2. Yes, this aria-label is used to clearly indicate that the link is intended to send an email.

Copy link
Contributor

@tblivet tblivet left a comment

Choose a reason for hiding this comment

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

Thank you for the update! It’s okay for me, but we need to rebase here. Yesterday, I changed the target branch to 2.x

@yannicka
Copy link
Contributor Author

Thank you for the update! It’s okay for me, but we need to rebase here. Yesterday, I changed the target branch to 2.x

Can you do that?

tblivet
tblivet previously approved these changes Feb 20, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Feb 20, 2026
@ingridusta ingridusta self-assigned this Feb 20, 2026
Copy link

@ingridusta ingridusta left a comment

Choose a reason for hiding this comment

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

Hi @yannicka,

I have tested your pr. No regression on keyboard nav + voice over.

It's a QA approveeeed ✅

Thanks for your contribution.

@ingridusta ingridusta removed their assignment Feb 20, 2026
@tblivet tblivet merged commit 5c66c1d into PrestaShop:2.x Feb 20, 2026
12 checks passed
@github-project-automation github-project-automation bot moved this from To be tested to Merged in PR Dashboard Feb 20, 2026
@ps-jarvis ps-jarvis moved this from Merged to To be tested in PR Dashboard Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants