[redesign] implementing tabs interaction #157
Conversation
There was a problem hiding this comment.
Please confirm that you've implemented this in an accessible way. It is a requirement of this project that it is implemented in an accessible way from the ground up.
You can reference https://www.w3.org/TR/wai-aria-practices/examples/tabs/tabs-1/tabs.html.
mansona
changed the title
|
@MelSumner do you have any objection to us creating an issue to make this component accessible as a follow-up issue instead of blocking the merge into the redesign branch? I understand your position if we were merging to master and agree 👍 |
|
@mansona yes, I object. Accessibility needs to be tested before we allow anything new. If we don't test for accessibility first, then it's an afterthought, and we already know that it causes a lot of issues to approach it that way (which is why we chose differently). |
| @@ -0,0 +1,9 @@ | |||
| <div class="tabs-item {{if this.active 'tabs-item--active'}}"> | |||
| <a class="tabs-link" href="javascript:void(0)" onclick={{action 'click'}}>{{@title}}</a> | |||
There was a problem hiding this comment.
this should be a button not a link
|
|
||
| {{#if this.active}} | ||
| <EmberWormhole @to={{@wormholeId}}> | ||
| <p>{{yield}}</p> |
There was a problem hiding this comment.
a <div> should wrap this content, not a <p>
|
@mansona there are also some merge conflicts to be resolved, thank you! |
|
@MelSumner, @mansona, what's the status of this (and the related #158)? I could potentially help out if you're still looking to update the tabs in an accessible way. Regards, -S |
|
@steveax Yes it still needs to be done, I guess I felt a little bit blocked on it because it was an intimidating task to achieve. Are you on the Ember Community Discord? could you reach out to me there (my nick is @real_ate) and we can coordinate the work 😄 |
|
The new tabs component has been removed because it is no longer needed by the design so I'm closing this PR for now. We can revive it later if we feel like it needs to be 👍 |
@ghislaineguerin recently added "tabs" styles and this PR just adds functionality to those tabs.
I'm doing it as a bit of a contextual component to make it as easy as possible for people to use (at the expense of making it a bit more complicated than it probably ought to be)
@MelSumner I'm wondering about the accessibility of this component 🤔 should we make sure that it is accessible before merging it or should we create an issue to review it and fix it (so we can keep up the momentum)?