search

mobify / stencil

DEPRECIATED - The latest Stencil development is currently taking place in the Adaptive.js repo.
MIT License
4 stars 0 forks source link

Tabs Component #110

Closed cole-sanderson closed 9 years ago

cole-sanderson commented 9 years ago

This PR adds the Tabs component

Tabs component is built on top of vellum (add-vellum branch)

Status: Read for Review Reviewers: @kpeatt @ry5n @jeffkamo @mlworks @nastiatikk @avelinet

Todo

cole-sanderson commented 9 years ago

To do: add JS to make tabs component to work.

jeffkamo commented 9 years ago

Hey @cole-sanderson something I noticed on @mlworks and @ry5n's work on their tab component is that they use role="tab" and role="tabpanel" differently (probably correctly):

They should actually be used like...

<button class="c-tabs__button" role="tab" ...>...

and

<section class="c-tabs__content {class}" ... role="tabpanel">...
ry5n commented 9 years ago

One thing to take a look at for accessibility is the tabs example from http://a11yproject.com/.

The other (unrelated) thing is that I don’t really like tying the tab controls to their content. I quite like the way that the Polymer tabs component is implemented, which is that the tab controls are one component, and they can be used to control any number of other things. For us, that could be something like Scooch. Food for thought anyway.

kpeatt commented 9 years ago

I quite like the way that the Polymer tabs component is implemented, which is that the tab controls are one component, and they can be used to control any number of other things. For us, that could be something like Scooch. Food for thought anyway.

:+1:

kpeatt commented 9 years ago

This would pair nicely with the 'toggle' standard UI script.

jeffkamo commented 9 years ago

Interesting idea, I like it!

cole-sanderson commented 9 years ago

Thanks guys for great inputs, we will need to get engi to add some JS to tabs.

jeffkamo commented 9 years ago

@cole-sanderson good work, although I think you should totally take a stab at the JS :)

You can use the link that Ryan posted as a starting point.

ry5n commented 9 years ago

This is a great start. Most of the question marks we have are with the JS implementation, and that would be better handled within the framework we’re developing for Stencil 2.0. @cole-sanderson ultimately we should move this to a new repo → stencil-tabs. You can ping @kpeatt to create it. Until then, I’d encourage work on the JS for this in this branch, but I’m going to close the PR for now.

kpeatt commented 9 years ago

@ry5n instead of closing PRs here should we move them under a specific milestone that says whether they're ready for a 2.0 port?

ry5n commented 9 years ago

Not sure. I think this one needs more work – we need to figure out whether the tabs should be two things or one and write the JS.

ry5n commented 9 years ago

Adding labels for issue status. The main tasks here are: