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

Bug - Table - Adding screenReaderText to a empty Th element changes column width #6643

Open
1 of 3 tasks
mvollmer opened this issue May 10, 2024 · 1 comment
Open
1 of 3 tasks
Assignees
Labels
bug Something isn't working

Comments

@mvollmer
Copy link

Adding screenReaderText to a empty Th element changes the min-width property of the element, which might change column width.

We have a table with a empty Th for the first column. That column contains icons:

image

The code for the header is pretty straighforward:

                    <Thead>
                        <Tr>
                            { show_icons && <Th /> }
                            <Th>{_("ID")}</Th>
                            <Th>{_("Type")}</Th>
                            <Th>{_("Location")}</Th>
                            <Th className="storage-size-column-header">{_("Size")}</Th>
                            <Th />
                        </Tr>
                    </Thead>

But we have started to receive this warning recently:

Th: Table headers must have an accessible name. If the Th is
intended to be visually empty, pass in screenReaderText. If the Th
contains only non-text, interactive content such as a checkbox or
expand toggle, pass in an aria-label.

The correct thing to do seems to be to add screenReaderText="Category" to make the first column more accessible. This however results in a changed layout:

image

The first column is now much wider than it needs to be.

This happens because Patternfly has a CSS rule that sets the min-width property of elements inside thead to some largish value, except when the element is empty according to the :empty pseudo-class, in which case the min-width property is zero. Our layout relies on this exception.

Giving screenReaderText a value will add a invisible element into the Th, which causes the "min-width: 0" exception to no longer apply.

I had expected that adding screenReaderText does not influence the layout. We will have to use aria-label instead, I guess.

This is an issue with

  • Patternfly 5
  • Patternfly 6
  • other
@mvollmer mvollmer added the bug Something isn't working label May 10, 2024
mvollmer added a commit to mvollmer/cockpit-machines that referenced this issue May 14, 2024
As requested by this warning:

    Th: Table headers must have an accessible name. If the Th is
    intended to be visually empty, pass in screenReaderText. If the Th
    contains only non-text, interactive content such as a checkbox or
    expand toggle, pass in an aria-label.

We can't use screenReaderText because of

    patternfly/patternfly#6643
mvollmer added a commit to mvollmer/cockpit-machines that referenced this issue May 14, 2024
As requested by this warning:

    Th: Table headers must have an accessible name. If the Th is
    intended to be visually empty, pass in screenReaderText. If the Th
    contains only non-text, interactive content such as a checkbox or
    expand toggle, pass in an aria-label.

We can't use screenReaderText because of

    patternfly/patternfly#6643
mvollmer added a commit to mvollmer/cockpit-machines that referenced this issue May 14, 2024
As requested by this warning:

    Th: Table headers must have an accessible name. If the Th is
    intended to be visually empty, pass in screenReaderText. If the Th
    contains only non-text, interactive content such as a checkbox or
    expand toggle, pass in an aria-label.

We can't use screenReaderText because of

    patternfly/patternfly#6643
@mattnolting
Copy link
Contributor

Adding screenReaderText to a empty Th element changes the min-width property of the element, which might change column width.

This happens because Patternfly has a CSS rule that sets the min-width property of elements inside thead to some largish value, except when the element is empty according to the :empty pseudo-class, in which case the min-width property is zero. Our layout relies on this exception.

Correct, which means this declaration will be updated to accommodate screen reader text.

Screenshot 2024-05-16 at 11 41 05 AM

mvollmer added a commit to cockpit-project/cockpit-machines that referenced this issue May 21, 2024
As requested by this warning:

    Th: Table headers must have an accessible name. If the Th is
    intended to be visually empty, pass in screenReaderText. If the Th
    contains only non-text, interactive content such as a checkbox or
    expand toggle, pass in an aria-label.

We can't use screenReaderText because of

    patternfly/patternfly#6643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Needs info
Development

No branches or pull requests

2 participants