core icon indicating copy to clipboard operation
core copied to clipboard

CdsDropdown triggers an error related to the popup API

Open hippee-lee opened this issue 3 years ago • 2 comments

Describe the bug

When using cds-dropdown elements Chrome triggers the following error:

Found a 'popup' attribute. If you are testing the popup API, you must enable Experimental Web Platform Features. If not, note that custom attributes must start with 'data-': https://html.spec.whatwg.org/multipage/dom.html#custom-data-attribute. This usage will likely cause site breakage when the popup API ships: https://chromestatus.com/feature/5463833265045504.

The errors are not so bad when there are only one or two dropdowns. However when this is used with CdsGrid and the detail view, each row with a preview button triggers an error.

How to reproduce

  1. Go to the docs
  2. open dev tools console
  3. Observe the errors

Expected behavior

Chrome/edge will not throw errors when using the cds-dropdown element.

Versions

Clarity project:

  • [x] Clarity Core
  • [ ] Clarity Angular/UI

Clarity version:

  • [ ] v5.x
  • [x] v6.x

Device:

  • Type: [e.g. MacBook]
  • OS: [e.g. iOS]
  • Browser [e.g. chrome, edge]
  • Version [Chrome 104]

Additional notes

This feature is slated to ship in Chrome v110.

hippee-lee avatar Aug 11 '22 00:08 hippee-lee

Thanks @hippee-lee , I noticed this a week or so ago too, but didn't have a chance to investigate.

ashleyryan avatar Aug 15 '22 14:08 ashleyryan

Ok digging into this some more. Open UI has proposed adding a new popup attribute: https://open-ui.org/components/popup.research.explainer#html-content-attribute which is what chromium is implementing. The attribute would go on the element that is actually the popup (instead of the anchor in our case), and it would only have specific values. So I think we're going to have to rename this attribute, which would be a breaking change. However, the dropdown component is still in beta, so I think it's okay - or we can keep both and log a deprecation warning if you use popup...

According to the documentation, it sounds like you can put it on any element that is the anchor element and it will apply aria attributes for you. This would make me learn towards calling it cds-popup. In practice, you can only put it on elements that have the reactive controller, so any clarity component that extends CdsButton elements. In that case something less specific, like popup-element could work. But if we're going to change popup to popup-element, then perhaps we should also change trigger to trigger-element for consistency...

https://github.com/vmware-clarity/core/blob/main/projects/core/src/internal/controllers/aria-popup-trigger.controller.ts https://github.com/vmware-clarity/core/blob/main/projects/core/src/internal/controllers/aria-popup.controller.ts

ashleyryan avatar Aug 15 '22 17:08 ashleyryan

:tada: This issue has been resolved in version 6.2.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Oct 24 '22 20:10 github-actions[bot]

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.

github-actions[bot] avatar Nov 08 '22 00:11 github-actions[bot]