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

Status of dropdownId arg? #673

Open
nwhittaker opened this issue Jul 29, 2022 · 4 comments
Open

Status of dropdownId arg? #673

nwhittaker opened this issue Jul 29, 2022 · 4 comments

Comments

@nwhittaker
Copy link

The <BasicDropdown> component includes a way to override the <BasicDropdownContent> component's id attribute:

But it's undocumented on https://ember-basic-dropdown.com/docs/api-reference. Is it safe to use? If so, it looks like there are some bugs in the {{basic-dropdown-trigger}} modifier where the default id is hard-coded:

element.setAttribute('aria-owns', `ember-basic-dropdown-content-${dropdown.uniqueId}`);
element.setAttribute('aria-controls', `ember-basic-dropdown-content-${dropdown.uniqueId}`);

Specifying (or somehow acquiring) the id is useful in cases where focus needs to be trapped in the drop-down.

@Techn1x
Copy link
Contributor

Techn1x commented Aug 26, 2024

Hello! I had a hand in writing the modifier. It was a while ago, I think I just opted to use the uniqueId present in the "publicAPI" since that is yielded. Whereas dropdownId isn't really documented (like you found) nor was it clear to me what it is doing.

At a glance I think dropdownId was intended for selecting the dropdown content and I think it might be leftover code from before uniqueId was added. The dropdown likely only works when its dropdownId arg is left undefined, so we should probably remove it as an argument..

Whereas uniqueId is a guid for the main component, that is intended to be used in the strings for the ids of both trigger & content.

Specifying (or somehow acquiring) the id is useful in cases where focus needs to be trapped in the drop-down.

If it helps, the uniqueId is yielded as part of the "publicAPI"... though it is just the guid, rather than the full ember-basic-dropdown-content-${uniqueId} so it's likely not that helpful unless you just need some kind of unique reference for this dropdown to do some logic with

How are you aiming to apply the focus trap? Could you apply it perhaps as a modifier on the content element? The focus-trap modifier below would be able to get the element id that you're after

<BasicDropdown as |dd|>
  <button {{basic-dropdown-trigger @dropdown=dd}}>trigger</button>
  <dd.Content {{focus-trap}}>content </dd.Content>
</BasicDropdown>

@Techn1x
Copy link
Contributor

Techn1x commented Aug 26, 2024

And I've now just realised that this post I replied to is from 2022 😄 ah well, hope my response is helpful regardless

@nwhittaker
Copy link
Author

And I've now just realised that this post I replied to is from 2022 😄 ah well, hope my response is helpful regardless

Yeah, the code I was maintaining is long gone. Thanks for looking into it anyway 😅.

Regarding the focus-trap: It was the ember-focus-trap modifier where I believe I was looking for a way to streamline setting its passthrough fallbackFocus option.

@mkszepp
Copy link
Collaborator

mkszepp commented Aug 29, 2024

@nwhittaker i agree @Techn1x, its not documented and i think it isn't working anymore like it was on begin...

Looking into the history the dropdownId was introduced in year 2016 together with triggerId.
The triggerId was already removed, the dropdownId was maybe always forget.

It looks like many years ago (2016) the uniqueId was introduced instead, which is present in public api...

As there is no test... i think we can deprecate it in v8 and we will remove in next major... i will look to prepare this in next time

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

No branches or pull requests

3 participants