Missing role attributes in Core 5.6.1 #6525

astorije · 2021-12-20T11:48:48Z

Describe the bug

ARIA roles and attributes are missing in Clarity Core v5.6.1, failing our React Testing Library tests that use await screen.findByRole('button').

How to reproduce

Steps to reproduce the behavior:

I could not make this work with Stackblitz or Code Sandbox, so had to go the repro repo route: https://github.com/astorije/clarity-missing-role
Clone, install dependencies (yarn install)
Run yarn test on the main branch and see the error in the console output
Switch to the core-5.6.0 branch, install dependencies again (yarn install), and run yarn test to see the same tests pass

Here is the error output on v5.6.1:

 FAIL  src/App.test.tsx
  App
    ✕ foo? (1042 ms)

  ● App › foo?

    Unable to find role="button"

    Ignored nodes: comments, <script />, <style />
    <body
      cds-supports="js no-flex-gap"
      cds-version="5.6.1"
    >
      <div>
        <div
          cds-layout="m:md"
        >
          <cds-button
            _nfg=""
            action="solid"
            size="md"
            status="primary"
            tabindex="0"
          >
            Foo
          </cds-button>
        </div>
      </div>
    </body>

      10 |
      11 |     expect(
    > 12 |       await screen.findByRole("button", { name: "Foo" })
         |                    ^
      13 |     ).toBeInTheDocument();
      14 |   });
      15 | });

As you can tell, the cds-button element is missing the role attribute (and the aria-disabled one).
I suspect the issue originated at v5.6.0...v5.6.1#diff-40c8e7c2cb3e2e864efc9529dbba5def804aa9131b0ab70c82fb87cfc86aba00
(warning: GitHub's renderer currently removes the # between v5.6.1 and diff in this URL 🤷‍♂️)

Expected behavior

The core-5.6.0 contains the expected behavior. When running tests, we get:

 PASS  src/App.test.tsx
  App
    ✓ foo? (144 ms)

  console.log
    <body
      cds-supports="js no-flex-gap"
      cds-version="5.6.0"
    >
      <div>
        <div
          cds-layout="m:md"
        >
          <cds-button
            _nfg=""
            action="solid"
            aria-disabled="false"
            role="button"
            size="md"
            status="primary"
            tabindex="0"
          >
            Foo
          </cds-button>
        </div>
      </div>
    </body>

    /.../src/App.test.tsx:15:12
      13 |     ).toBeInTheDocument();
      14 |
    > 15 |     screen.debug();
         |            ^

Versions

Clarity project:

Clarity Core
Clarity Angular/UI

Clarity version:

v3.x
v4.x
v5.6.1

Framework:

Angular
React
Vue
Other:

Additional notes

To be noted that the role does appear fine in the browser but not in the test DOM, not sure what kind of magic is happening there:

The text was updated successfully, but these errors were encountered:

coryrylan · 2022-01-03T17:06:41Z

We use AOM aria properties within the components. Are you using Jest or JSDom? It could be they don't support AOM

astorije · 2022-01-03T17:14:53Z

We're using Jest and JSDom by way of React Testing Library.
Was AOM being used before v5.6.1? Everything worked fine in v5.6.0 and tests started failing as of v5.6.1 only.

URL above has an GitHub rendering issue, but I suspected this change caused this:

coryrylan · 2022-01-05T16:12:44Z

We have been working on moving away from overriding properties on the native HTMLElement prototype since it can cause unexpected behaviors in some browsers as well as performance issues. Some web component tools actively try to prevent this https://github.com/ionic-team/stencil/blob/main/src/compiler/transformers/reserved-public-members.ts

astorije · 2022-01-06T14:26:07Z

Good to know, thank you!
Any recommendations you could provide us on how to move forward here? We're kinda stuck between a rock and a hard place on that one as well.
If all else fails, we can mock the components as a last resort, though we try to avoid this as much as we can.

ashleyryan · 2022-01-21T17:45:02Z

I filed an issue again jsdom to support the AOM properties: jsdom/jsdom#3323

We have a polyfill that will fall back to setAttribute for browsers that don't support the property (currently just Firefox), but it doesn't run in node because there were issues with jsdom.

Fixes vmware-archive#5985 - Add isNode check to property syncing in cdscontrol. Fixes vmware-archive#6525: - Initialize role properties in connectedCallback. - Remove isNode check for aria-reflect. Fixes Signed-off-by: Ashley Ryan <[email protected]>

Fixes vmware-archive#5985 - Add isNode check to property syncing in cdscontrol. Fixes vmware-archive#6525: - Initialize role properties in connectedCallback. - Remove isNode check for aria-reflect. Signed-off-by: Ashley Ryan <[email protected]>

Fixes vmware#5985 - Add isNode check to property syncing in cdscontrol. Fixes vmware#6525: - Remove isNode check for aria-reflect, now that all roles are set in connectedCallback Signed-off-by: Ashley Ryan <[email protected]>

Fixes #5985 - Add isNode check to property syncing in cdscontrol. Fixes #6525: - Remove isNode check for aria-reflect, now that all roles are set in connectedCallback Signed-off-by: Ashley Ryan <[email protected]>

Fixes vmware#5985 - Add isNode check to property syncing in cdscontrol. Fixes vmware#6525: - Remove isNode check for aria-reflect, now that all roles are set in connectedCallback Signed-off-by: Ashley Ryan <[email protected]>

github-actions · 2022-02-19T00:05:26Z

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

coryrylan added the external tooling label Jan 3, 2022

ashleyryan mentioned this issue Jan 31, 2022

fix(react): fix jsdom react testing issues #6612

Merged

14 tasks

ashleyryan closed this as completed in #6612 Feb 4, 2022

ashleyryan mentioned this issue Feb 4, 2022

[Backport] fix(react): fix jsdom react testing issues #6636

Closed

14 tasks

ashleyryan mentioned this issue Feb 4, 2022

[Backport] fix(react): fix jsdom react testing issues #6640

Merged

14 tasks

github-actions bot locked and limited conversation to collaborators Feb 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Missing role attributes in Core 5.6.1 #6525

Missing role attributes in Core 5.6.1 #6525

astorije commented Dec 20, 2021 •

edited

Loading

coryrylan commented Jan 3, 2022 •

edited

Loading

astorije commented Jan 3, 2022

coryrylan commented Jan 5, 2022

astorije commented Jan 6, 2022

ashleyryan commented Jan 21, 2022 •

edited

Loading

github-actions bot commented Feb 19, 2022

Missing role attributes in Core 5.6.1 #6525

Missing role attributes in Core 5.6.1 #6525

Comments

astorije commented Dec 20, 2021 • edited Loading

Describe the bug

How to reproduce

Expected behavior

Versions

Additional notes

coryrylan commented Jan 3, 2022 • edited Loading

astorije commented Jan 3, 2022

coryrylan commented Jan 5, 2022

astorije commented Jan 6, 2022

ashleyryan commented Jan 21, 2022 • edited Loading

github-actions bot commented Feb 19, 2022

astorije commented Dec 20, 2021 •

edited

Loading

coryrylan commented Jan 3, 2022 •

edited

Loading

ashleyryan commented Jan 21, 2022 •

edited

Loading