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

Dynamic Root Elements #38

Closed
chadian opened this issue Jun 5, 2020 · 4 comments Β· Fixed by #46
Closed

Dynamic Root Elements #38

chadian opened this issue Jun 5, 2020 · 4 comments Β· Fixed by #46
Labels
enhance: code Issue asks for new feature or refactor

Comments

@chadian
Copy link

chadian commented Jun 5, 2020

I would like to see... πŸ™‹β€β™€οΈπŸ™‹β€β™‚οΈ

Based on our discussion @ijlee2, it might be a useful feature to see the possibility of customizing the root element of the container.

Why and how πŸ’¬

How: It looks like this is possible now and is considered stable based on ember-element-helper. The compatibility might be limited to Ember 3.11 and up if the modifier needs to be attached to the element.

Why: Keeping markup semantic and for cases where the markup element is important (possibly for css selectors)

@chadian chadian added the enhance: code Issue asks for new feature or refactor label Jun 5, 2020
@ijlee2
Copy link
Owner

ijlee2 commented Jun 5, 2020

Thanks for writing up the issue, @chadian. I agree that it's important to allow a dynamic tag instead of <div> to facilitate accessibility and semantic HTML.

Because I limited the addon support to Ember 3.16+ (and conjectured that the addon may work for 3.12-3.15), we should be okay with using ember-element-helper to solve the problem.

Potential problem

Today, I read the README and RFC #389 to understand the ramifications of allowing a dynamic tag. The RFC has a section called How we teach this. It suggests that a developer should follow the WAI-ARIA specification (e.g. assign the right role) when changing the tag.

I'm okay with allowing ...attributes to be used to help a developer follow the WAI-ARIA spec, in addition to its original, sole purpose of helping the developer pass class or local-class.

I think, in this addon's README, we will need to add a warning that changing the tag and passing more things (possibly in the right order) to ...attributes should be done with care.

Action items

If you'd like, let's start working on this next week (whenever you are free).

As I don't have much experience with accessibility, I think it'd be good to reach out to the Ember A11y team on Discord (at #topic-a11y channel). I'd like to know:

  • Can a developer use any tag, or are there tags that we should disallow, in the context of container queries (an element resize)?
  • Is using ...attributes to allow a developer to pass things to meet WAI-ARIA spec the right Ember Octane way? Or is there an alternative approach that better separates the boundary between the <ContainerQuery> component and the consuming component?

Please let me know what you think and questions that you may have. Thanks again for your help!

@ijlee2
Copy link
Owner

ijlee2 commented Jun 20, 2020

Reference(s), mainly for myself so that I don't slack ☺️

Right now, the best way to do it is for the developer to be aware of the necessary attributes and add them on their own. There are aria- attributes that can have multiple values and the order of those values matter, and ...attributes can't guarantee that right now (but I think @chancancode and @wycats have some ideas about how that could improve)

In total, there are five aria- attributes that can have multiple values:

  • aria-controls order could matter in some instances.
  • aria-describedby order definitely matters
  • aria-flowto the order could matter, depending on intent of content.
  • aria-labelledby order definitely matters
  • aria-owns but if you see this in code, do a close code review or some extra research because it's a code smell

It [the order] can't be guaranteed, which means some extra coordination would be necessary to avoid some a11y-related bugs. But it's still useful.

@chadian
Copy link
Author

chadian commented Jun 23, 2020

Thanks for taking the lead on figuring out the accessibility piece to having dynamic root elements. I don't currently have the time to jump in on implementation for at least 3 more weeks but will be watching this issue and will contribute where I can. I am very much looking forward to seeing this land though so I will circle back.

@ijlee2
Copy link
Owner

ijlee2 commented Jun 23, 2020

@chadian I think I can work on trying out ember-element-helper next week. Trying to take things easy and not code too much during the covid outbreak. (Trying but failing πŸ™‚)

After merging the PR for this issue, we'll probably want to look at how we can bring a solution back to ember-fill-up.

@ijlee2 ijlee2 linked a pull request Jul 3, 2020 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhance: code Issue asks for new feature or refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants