Define capabilities and API for sl-tab-panel

notmessenger commented 8 years ago

This issue is where discussion about all of the things we want to change about the sl-tab-panel component should occur and the final definition of its capabilities and API are defined. As these items are discussed and vetted, issues will be created to work any individual tasks, with this issue serving as the overall source of progress.

To indicate that a capability of the component needs to be verified, add a comment with **VERIFY** as the first line. To propose a change to the component, add a comment with **PROPOSAL** as the first line. To indicate that something needs to be researched, add a comment with **RESEARCH** as the first line.

As you are leaving comments regarding any of these VERIFY, PROPOSAL, or RESEARCH comments, link back to the comments link in your own so that the conversation can be more easily followed (in absence of threaded conversation support in Github). This should be done on the first line of the comment via **RE: <url>**

Capabilities and API

[ ] ARIA support
[ ] ...

Tasks Adding them here until issues are created

Update documentation for each change
Complete review of documentation against capabilities

Issues tracking these efforts

https://github.com/softlayer/sl-ember-components/issues?q=is%3Aopen+is%3Aissue+milestone%3A%22v0.13.0+%28Final+push+for+a+1.0.0+release%29%22+label%3Asl-tab-panel

SpikedKira commented 8 years ago

PROPOSAL

Add support for pills and nav-stacked: http://getbootstrap.com/components/#nav-pills

notmessenger commented 8 years ago

PROPOSAL

Allow for the binding of an action to tab selection so can react to a new tab being displayed. Depending on Bootstrap's capabilities this should include all possible events (i.e. beforeTabShow, tabShown, etc)

azizpunjani commented 8 years ago

PROPOSAL

Since nav in bootstrap is independent and can be used without corresponding togglable tabs, create a new component called sl-nav. sl-nav can then be used within the sl-tab-panel and it can also be used without being in an sl-tab-panel just like in bootstrap docs.

{{#sl-tab-panel}}
    {{#sl-nav}}
        {{#sl-nav-item}} Home {{/sl-nav-item}}
        {{#sl-nav-item}} Work {{/sl-nav-item}}
    {{/sl-nav}}

    {{#sl-tab-content}}
        {{#sl-tab-pane}} 
           This is info about my home
        {{/sl-tab-pane}}

        {{#sl-tab-pane}} 
           Here is info about my work
        {{/sl-tab-pane}}
    {{/sl-tab-content}}
{{/sl-tab-panel}}

The sl-nav component accepts a type which is tabs by default if one is not set. The type can be also be pills or stacked. sl-nav also accepts a justified boolean property which adds the nav-justified class.

{{#sl-nav type="pills"}}
    ...
{{/sl-nav}}

We would also create a new sl-tab-content component that will be used as a container for `sl-tab-pane``s

erangeles commented 8 years ago

PROPOSAL

Currently at the moment we are setting active manually in the sl tab, however we DO NOT set it for sl pane but active is still showing up on sl pane when the corresponding sl tab is selected/active. Let's either remove the manual setup or also manually set up the active prop in sl-pane.

azizpunjani commented 8 years ago

Proposal V2

Another proposal to keep in mind if V1 of my proposal is not a good fit.

Background

The sl-tab-panel is used in conjunction with multiple tab panes i.e sl-tab-pane. Currently there's a lot of DOM manipulation code within the sl-tab-panel, the reason for this code is because the parent does not have any knowledge of the children and hence has to drop down to the DOM level and use jQuery to select the children and use the resultant data for setting up the navigation tabs.

Proposed solution

Using contextual components and closure actions we would pass a registerPane action in the background, the tab pane would register itself onto the parent panel component. We would then have access to each one of the panes within the panel component and no longer need to rely on DOM traversal/manipulations.

Example code

sl-tab-panel.js

actions: {
    registerPane( pane ) {
        const panes = this.get( 'panes' );
        panes.push( pane );
    }
 }

sl-tab-pane.js

init() {
    this._super( ...arguments );
    this.get( 'registerPane' )( this );
}

sl-tab-panel.hbs

...
<div class="tab-content">
    {{yield (hash pane=(component 'sl-tab-pane' registerPane=(action "registerPane")))}}
</div>

How the consuming user would use it:

{{#sl-tab-panel as |panel|}}
    {{#panel.pane label="Two" name="two"}}
        <p>Content for tab two!</p>
    {{/panel.pane}}

    {{#panel.pane label="Three" name="three"}}
        <p>Content for tab three!</p>
    {{/panel.pane}}
{{/sl-tab-panel}}

Pros

This is arguably a more "Ember" way of doing things instead of directly traversing the DOM.
It cleans up quite a bit of code that is currently having to get attributes off the children.

Cons

The user has to get used to a new panel.pane syntax.

softlayer / sl-ember-components