KittyGiraudel/a11y-dialog

Provide a different data attribute than `id`

roblevintennis opened this issue · 5 comments

This script and clear documentation is so cool!

I was quite please to see the use of a data attribute was available in place of an ID (obviously IDs have the only one per page collision problem)! However, I see that the decision to use ID if you need a handle or data attribute for automatic detection.

I feel there's really a compelling need for a third possibility to use a data attribute (or really any valid selector), but also be able to get a handle to the API.

I think the fact that a handle to the API is required to inject a scroll lock on the body makes this flexibility even more crucial (if you're really hoping to avoid the use of ID).

I think we could add some sort of selector prop:
const attrSelector = NEWPROP-API-PROPOSAL || 'data-a11y-dialog'

So if exists we use else fallback to what you already have and then adjust initialize line:

this._id = this.$el.getAttribute(attrSector) || this.$el.id

My assumption is the one concern here might be increasing the surface area of the API. I think this could be addressed by either having it undocumented or better you just include it within the same documentation for the data-a11y-dialog attribute you already have mentioning the default behavior, but also mentioning how to override the selector should you need the API handle.

All to say, if you think this idea has merit I would be happy to submit a pull request! LMK thanks.

Hi! Just to see if I get that right: you would like to be able to use a data attribute instead of an id but not use auto-instantiation. Is that correct?

Essentially, yes :)

Alright, I’ll think about it. :)

I just reread https://a11y-dialog.netlify.app/usage/instantiation and I'm realizing this is actually already be possible more or less if the consumer just uses their own query selector and passes the element wholesale like:

const element = document.querySelector("[data-foo='bar-baz']");
const dialog = new A11yDialog(element)

I think this was just my initial misunderstanding of the what's available in the API so I'll go ahead close this issue.

Oh, I’m glad this is resolved!