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.

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

Addition of role='navigation' to the <ul> element causes a Lighthouse failure #459

Open
stevejay opened this issue Nov 30, 2022 · 12 comments · May be fixed by #494
Open

Addition of role='navigation' to the <ul> element causes a Lighthouse failure #459

stevejay opened this issue Nov 30, 2022 · 12 comments · May be fixed by #494

Comments

@stevejay
Copy link

stevejay commented Nov 30, 2022

If I run Lighthouse on our pagination component, Lighthouse complains that List items (<li>) are not contained within <ul> or <ol> parent elements.:

Screenshot 2022-11-30 at 19 12 09

AFAIK the 'navigation' role overrides the natural 'list' role of the <ul> element.

The addition of this role was a recent change, and the release notes link to this page to explain its addition. However, that page shows the use of the navigation role as wrapping the <ul> element, not added to it:

<div role="navigation" aria-label="Customer service">
  <ul>
    <li><a href="#">Help</a></li>
    <li><a href="#">Order tracking</a></li>
    <li><a href="#">Shipping &amp; Delivery</a></li>
    <li><a href="#">Returns</a></li>
    <li><a href="#">Contact us</a></li>
    <li><a href="#">Find a store</a></li>
  </ul>
</div>

Is there some way to avoid this role being added? It would be preferable if it was configurable.

@devinschulz
Copy link

Getting a similar error when using axe:

 "<li> elements must be contained in a <ul> or <ol> (listitem)"

    Fix any of the following:
      List item does not have a <ul>, <ol> parent element without a role, or a role="list"

    You can find more information on this issue here:
    https://dequeuniversity.com/rules/axe/4.5/listitem?application=axeAPI

The solution @stevejay provided would fix this accessibility issue. Another option would be to wrap the unordered list in a nav tag and remove the role attribute altogether.

@racingfoxs
Copy link

racingfoxs commented Feb 2, 2023

Same problem facing!

How to I change/assign

  • role?

    <ReactPaginate nextLabel=">" onPageChange={handlePageClick} pageRangeDisplayed={3} marginPagesDisplayed={2} pageCount={pageCount} previousLabel="<" pageClassName="inline-flex items-center px-4 py-2 text-sm font-bold border dark:border-gray-700" pageLinkClassName="" previousClassName="inline-flex items-center px-6 py-2 text-sm font-bold border rounded-l-md dark:border-gray-700" previousLinkClassName="" nextClassName="inline-flex items-center px-6 py-2 text-sm font-bold border rounded-r-md dark:border-gray-700" nextLinkClassName="" breakLabel="..." breakClassName="" breakLinkClassName="" containerClassName="" activeClassName="inline-flex items-center px-4 py-2 text-sm font-semibold border dark:bg-violet-400 dark:text-gray-900 dark:border-gray-700" renderOnZeroPageCount={null} />

  • @racingfoxs
    Copy link

    Update:- I was trying to fix myself and found a solution.
    Solution:- Wrap under <nav> Tag Having aria-label="Pagination".
    Example:-

    <nav aria-label="Pagination">
        <ReactPaginate/>
    </nav>
    

    @amitt2202
    Copy link

    amitt2202 commented Feb 22, 2023

    The solution mentioned by @racingfoxs didn't work. the <ul> element still comes with role="navigation" which is complainaed by the axe tool. I did make the changes to the file "PaginationBoxView.js" present in .....\node_modules\react-paginate\react_components folder line number 566 where <ul> tag is added with role="navigation" is also not working. Can i request the developers to provide alternative release without role="navigation" tag for <ul> element?
    this makes it fail the accessibility test

    @SLain123
    Copy link

    SLain123 commented Mar 6, 2023

    The solution mentioned by @racingfoxs didn't work. the <ul> element still comes with role="navigation" which is complainaed by the axe tool. I did make the changes to the file "PaginationBoxView.js" present in .....\node_modules\react-paginate\react_components folder line number 566 where <ul> tag is added with role="navigation" is also not working. Can i request the developers to provide alternative release without role="navigation" tag for <ul> element? this makes it fail the accessibility test

    +1. I tried to added "nav" wrapper with arai-label, but problem still actual.

    @TravelPaul
    Copy link

    TravelPaul commented Mar 7, 2023

    Having same issue, looks like the role for a ul needs to be list. Also tried wrapping in nav but doesn't change that the role "navigation" is being applied to the ul, which is the issue according to axe.

    Axe test is saying:

        Fix any of the following:
          List item does not have a <ul>, <ol> parent element without a role, or a role="list"
    

    using "jest-axe": "^5.0.1",
    and "react-paginate": "^8.1.4",

    So the fix as @stevejay would solve this.

    @mynamesleon
    Copy link

    Further to this, the aria-label on the generated <ul> isn't configurable. So the change that added this also leaves us with a hardcoded English "pagination" announcement, and without the ability to add additional attributes to that navigation landmark (such as aria-controls, if using the pagination for something that is dynamically updated).

    So I'm going to stick with 8.1.3 for now and wrap it in my own <nav> element that I can add aria-controls and a translated aria-label to.

    @kubaf13
    Copy link

    kubaf13 commented May 23, 2023

    Hey Hi Hello, any updates how to fix this issue?

    @amitt2202
    Copy link

    Hey Hi Hello, any updates how to fix this issue?

    Please use the fork : https://github.com/amitt2202/react-paginate-fts.git
    For my project i have made the fix and updated!

    @mynamesleon
    Copy link

    mynamesleon commented May 23, 2023

    Hey Hi Hello, any updates how to fix this issue?

    @kubaf13 The simplest option right now is to stick with v8.1.3 and avoid any newer versions.

    Despite the above comment, although @amitt2202's fork has removed the role attribute that was causing the issue, they've also added a hardcoded aria-label="Pagination123" onto the generated <ul> which can't be easily overriden by a consuming app.

    @kroney kroney linked a pull request Jul 31, 2023 that will close this issue
    @quicksketch
    Copy link

    +1 to the PR at #494 that fixes this issue. Thanks @kroney for making it.

    For others stuck in the same situation, our project decided to use patch-package to modify the dist/react-paginate.js and react_components/PaginationBoxView.js files while we wait for this fix to be applied.

    @thomcrielaard
    Copy link

    +1 to PR at #494

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging a pull request may close this issue.

    10 participants