testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.27k stars 467 forks source link

getByRole('caption') does not find caption element #1289

Closed Amy-B-Tradition closed 5 months ago

Amy-B-Tradition commented 9 months ago

image

16.20.2

Relevant code or config:

const BuyOrSellDepth = ({rows, type, securityId, currency, aggressCallback}: Props) => {
  const columns = depthColumnsByOrderType(type)
  const rootid = `${type}-market-${securityId}`
  const callback = aggressCallback || noop
  return (<table data-testid={rootid}>
    <caption >Current {orderTypeToDisplayName(type)}</caption>
    <tbody>{
      rows.map((row) => (<tr key={row.transactionId}
                             data-testid={`${rootid}-row-${row.transactionId}`}
        >
          {columns.map(col => {
            const key = `${rootid}-row-${row.transactionId}-${col}`
            let content: ReactNode = ''
            switch (col) {
              case 'aggress':
                if (row.canAggress) {
                  content = (
                    <span onClick={() => callback(row)}>
                      {orderTypeToAggressLabel(type)}
                    </span>
                  )
                }
                break
              case 'price':
                content = formatCurrency(row.price, currency)
                break
              case 'size':
                content = `${row.size}`
                break
              case 'spread':
                content = formatSpread(row.spread)
                break
            }

            return (<td
              key={key}
              data-testid={key}
              data-column={col}
            >
              {content}
            </td>)
          })}
        </tr>
      ))
    }</tbody>
  </table>)
}

What you did:

test:

it('should have a caption', () => {
      const { getByRole } = render(<BuyOrSellDepth rows={[]} securityId={12345} type={type} />)
      expect(getByRole('caption')).toHaveTextContent(`Current ${displayName}s`)
    })

What happened:

image

Reproduction:

I think maybe this is it, but codesandbox gets harder and harder to use all the time and I was not able to find how to run the tests anymore https://codesandbox.io/p/sandbox/react-testing-library-demo-forked-d8ztms?file=%2Fsrc%2F__tests__%2Fhello.js%3A10%2C1

Problem description:

According to the docs, caption should have an implicit caption aria role. You are not allowed to add an aria role to it, and even if you do, RTL errors out and doesn't find it

Suggested solution:

This should work

github-actions[bot] commented 9 months ago

Uh oh! @Amy-B-Tradition, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

github-actions[bot] commented 9 months ago

Uh oh! @Amy-B-Tradition, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

MatanBobi commented 9 months ago

Hi @Amy-B-Tradition, thanks for opening this one :) Any chance you can test this against our alpha release? (can be installed using npm install @testing-library/dom@alpha). We've updated the underlying package (aria-query) there. We're still running some tests and I didn't get a chance to push it forwards to a release but having someone else testing that release will really help us. Thanks!

Amy-B-Tradition commented 8 months ago

Oh, sorry, I just saw this. If you still need this, I'll try to make some time this weekend.

MatanBobi commented 8 months ago

Oh, sorry, I just saw this. If you still need this, I'll try to make some time this weekend.

As we still didn't ship the alpha version, any input will help. Thanks

AmyBlankenship commented 8 months ago

This didn't work for me, but I think it's probably because dom testing library is wrapped by react-testing-library and installing the alpha doesn't make it available. LMK if there's something you'd like me to try

MatanBobi commented 8 months ago

Yeah, you'll need to use something like npm's overrides or yarn's resolutions field to override a dependencies version :) Sorry, I forgot to mention that.

AmyBlankenship commented 8 months ago

Seems to work

MatanBobi commented 8 months ago

Thanks for testing this @AmyBlankenship! I'll do my best to find time to release this version soon though my plate is quite full at the moment.

AmyBlankenship commented 8 months ago

I appreciate your hard work!

jlp-craigmorten commented 7 months ago

v10.0.0 is now live with this update, and looks like react-testing-library v15.0.0 will drop soon, see https://github.com/testing-library/react-testing-library/pull/1295 (and then overrides won't be needed).

MatanBobi commented 5 months ago

I'm resolving this one as DTL and RTL are already released :) Thanks.