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

Tabs component #169

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ember-headlessui/addon/components/tabs-group.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<div data-test-ember-headlessui-tabs-wrapper ...attributes>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to have this wrapping div?

{{yield (hash
Title=(component 'tabs-group/-title' registerTabNames=this.registerTabNames selectTab=this.selectTab selectedTab=this.selectedTab)
Content=(component 'tabs-group/-content' selectedContent=this.selectedContent registerContent=this.registerContent))}}
</div>
36 changes: 36 additions & 0 deletions ember-headlessui/addon/components/tabs-group.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
export default class TABSGROUP extends Component {
tabNames = [];
Contents = [];
@tracked currentTab = null;
@action
registerTabNames(e) {
this.tabNames = [...this.tabNames, e];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using immutability patterns, can we switch to a TrackedArray? that will provide more optimal updates and not invalidate the whole collection

}
@action
registerContent(e) {
this.Contents = [...this.Contents, e];
this.currentTab = 0;
}
@action
selectTab(changedTo, changedFrom) {
this.currentTab = this.tabNames.indexOf(changedTo.target);
this.pastTab = this.tabNames.indexOf(changedFrom);
if (this.args.onChange) {
return this.args.onChange(
this.currentTab,
this.tabNames[this.currentTab],
this.tabNames[this.pastTab]
);
}
}

get selectedTab() {
return this.tabNames[this.currentTab];
}
get selectedContent() {
return this.Contents[this.currentTab];
}
}
7 changes: 7 additions & 0 deletions ember-headlessui/addon/components/tabs-group/-content.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<div class="{{if this.renderContent '' 'hidden'}}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can assume a "hidden" class exists

data-test-headlessui-tabs-content='test-tabs-content-{{@content-index}}'
{{this.registerContent}} ...attributes>
{{#if this.renderContent}}
{{yield}}
{{/if}}
</div>
15 changes: 15 additions & 0 deletions ember-headlessui/addon/components/tabs-group/-content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';

import { modifier } from 'ember-modifier';

export default class Content extends Component {
@tracked element = null;
get renderContent() {
return this.element === this.args.selectedContent;
}
registerContent = modifier((e) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not have this side-effect here? I know this pattern is common in headlessui, but it's a problematic pattern, and we don't want to propagate it.

Derived data will be more performant, have fewer layout shifts, and will in general be way easier to debug.

this.element = e;
this.args.registerContent(e);
});
}
11 changes: 11 additions & 0 deletions ember-headlessui/addon/components/tabs-group/-title.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<button
type='button'
data-test-headlessui-tabs-title='test-tabs-title-btn-{{@index}}'
id={{guid}}
disabled={{if @disable true false}}
{{on 'click' this.selectTab}}
{{this.registerTabs}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this modifier please <3

...attributes
>
{{yield}}
</button>
22 changes: 22 additions & 0 deletions ember-headlessui/addon/components/tabs-group/-title.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { guidFor } from '@ember/object/internals';

import { modifier } from 'ember-modifier';

export default class Title extends Component {
guid = `${guidFor(this)}-tailwindui-tabs-title`;
@tracked element = null;

registerTabs = modifier((e) => {
this.element = e;
this.args.registerTabNames(e);
});

@action
selectTab(changedTo) {
let changedFrom = this.args.selectedTab;
return this.args.selectTab(changedTo, changedFrom);
}
}
1 change: 1 addition & 0 deletions ember-headlessui/app/components/tabs-group.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'ember-headlessui/components/tabs-group';
1 change: 1 addition & 0 deletions ember-headlessui/app/components/tabs-group/-content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'ember-headlessui/components/tabs-group/-content';
1 change: 1 addition & 0 deletions ember-headlessui/app/components/tabs-group/-title.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from 'ember-headlessui/components/tabs-group/-title';
18 changes: 18 additions & 0 deletions test-app/app/controllers/tabs/tabs-basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { tracked } from '@glimmer/tracking';
import Controller from '@ember/controller';
import { action } from '@ember/object';

export default class TabsTabsBasicController extends Controller {
@tracked activatedIndex = 0;
titles = ['cars', 'motor cycles', 'boats', 'planes'];
contents = [
'cars content',
'motor cycles content',
'boats content',
'planes content',
];
@action
onChange(activeIndex) {
this.activatedIndex = activeIndex;
}
}
19 changes: 19 additions & 0 deletions test-app/app/controllers/tabs/tabs-with-disable-state.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { tracked } from '@glimmer/tracking';
import Controller from '@ember/controller';
import { action } from '@ember/object';

export default class TabsTabsBasicController extends Controller {
disabledToBeIndex = 2;
@tracked activatedIndex = 0;
titles = ['cars', 'motor cycles', 'boats', 'planes'];
contents = [
'cars content',
'motor cycles content',
'boats content',
'planes content',
];
@action
onChange(activeIndex) {
this.activatedIndex = activeIndex;
}
}
21 changes: 21 additions & 0 deletions test-app/app/controllers/tabs/tabs-with-onchange-action.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { tracked } from '@glimmer/tracking';
import Controller from '@ember/controller';
import { action } from '@ember/object';

export default class TabsTabsBasicController extends Controller {
@tracked activatedIndex = 0;
titles = ['cars', 'motor cycles', 'boats', 'planes'];
contents = [
'cars content',
'motor cycles content',
'boats content',
'planes content',
];
@action
onChange(activeIndex, changedTo, changedFrom) {
alert(
`sure you want to change to ${changedTo.textContent.trim()} from ${changedFrom.textContent.trim()}?`
);
this.activatedIndex = activeIndex;
}
}
6 changes: 6 additions & 0 deletions test-app/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,10 @@ Router.map(function () {
this.route('radio-group', function () {
this.route('radio-group-basic');
});

this.route('tabs', function () {
this.route('tabs-basic');
this.route('tabs-with-onchange-action');
this.route('tabs-with-disable-state');
});
});
3 changes: 3 additions & 0 deletions test-app/app/routes/tabs/tabs-basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import Route from '@ember/routing/route';

export default class TabsBasicRoute extends Route {}
24 changes: 23 additions & 1 deletion test-app/app/templates/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@
</li>
</ul>
</li>
<li>
<h3 class='text-xl'>
Tabs
</h3>
<ul>
<li>
<LinkTo @route='tabs.tabs-basic'>
tabs (Basic)
</LinkTo>
</li>
<li>
<LinkTo @route='tabs.tabs-with-onchange-action'>
tabs (with custom on change)
</LinkTo>
</li>
<li>
<LinkTo @route='tabs.tabs-with-disable-state'>
tabs (with disabled state)
</LinkTo>
</li>
</ul>
</li>
</ul>
</div>
</div>
</div>
22 changes: 22 additions & 0 deletions test-app/app/templates/tabs/tabs-basic.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<div class='w-full max-w-md px-2 py-16 mx-auto'>
<TabsGroup class="flex flex-col space-x-1 rounded-xl p-1" @onChange = {{this.onChange}} as |tabs|>
<div class='flex space-x-1 rounded-xl bg-sky-600/20 p-1'>
{{#each this.titles as |tabName index|}}
<tabs.Title
@index={{index}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4 pointer-events-auto {{if (eq index this.activatedIndex) "bg-white shadow"}}'
>
{{tabName}}
</tabs.Title>
{{/each}}
</div>
<div class="mx-auto w-full max-w-md shadow-md px-4 py-4 rounded-xl bg-white mt-4">
{{#each this.contents as |content index|}}
<tabs.Content class="content" @content-index={{index}}>
<p>{{content}}</p>
</tabs.Content>
{{/each}}
</div>
</TabsGroup>
</div>

22 changes: 22 additions & 0 deletions test-app/app/templates/tabs/tabs-with-disable-state.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<div class='w-full max-w-md px-2 py-16 mx-auto'>
<TabsGroup class="flex flex-col space-x-1 rounded-xl p-1" @onChange = {{this.onChange}} as |tabs|>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on internalizing the onChange behavior by default?

can we abstract away the need for the consumer to configure this? (I think so!)

<div class='flex space-x-1 rounded-xl bg-sky-600/20 p-1'>
{{#each this.titles as |tabName index|}}
<tabs.Title
@index={{index}}
@disable={{if (eq index this.disabledToBeIndex) true}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4 pointer-events-auto {{if (eq index this.activatedIndex) "bg-white shadow"}}'
>
{{tabName}}
</tabs.Title>
{{/each}}
</div>
<div class="mx-auto w-full max-w-md shadow-md px-4 py-4 rounded-xl bg-white mt-4">
{{#each this.contents as |content index|}}
<tabs.Content class="content" @content-index={{index}}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the index? and compare via object identity or some other way? using indexes makes looping inefficient / invalidate more frequently than we need it to

<p>{{content}}</p>
</tabs.Content>
{{/each}}
</div>
</TabsGroup>
</div>
21 changes: 21 additions & 0 deletions test-app/app/templates/tabs/tabs-with-onchange-action.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<div class='w-full max-w-md px-2 py-16 mx-auto'>
<TabsGroup class="flex flex-col space-x-1 rounded-xl p-1" @onChange = {{this.onChange}} as |tabs|>
<div class='flex space-x-1 rounded-xl bg-sky-600/20 p-1'>
{{#each this.titles as |tabName index|}}
<tabs.Title
@index={{index}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4 pointer-events-auto {{if (eq index this.activatedIndex) "bg-white shadow"}}'
>
{{tabName}}
</tabs.Title>
{{/each}}
</div>
<div class="mx-auto w-full max-w-md shadow-md px-4 py-4 rounded-xl bg-white mt-4">
{{#each this.contents as |content index|}}
<tabs.Content class="content" @content-index={{index}}>
<p>{{content}}</p>
</tabs.Content>
{{/each}}
</div>
</TabsGroup>
</div>
113 changes: 113 additions & 0 deletions test-app/tests/integration/components/tabs-basic-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { click, render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';

module('Integration | Component | <Tabs>', function (hooks) {
setupRenderingTest(hooks);
test('Should render the tabs component', async function (assert) {
await render(hbs`
<div class='w-full max-w-md px-2 py-16 mx-auto'>
<TabsGroup class="flex flex-col space-x-1 rounded-xl p-1" @onChange = {{this.onChange}} as |tabs|>
<div class='flex space-x-1 rounded-xl bg-sky-600/20 p-1'>
<tabs.Title
@index={{1}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'
>
hello
</tabs.Title>
<tabs.Title
@index={{2}}
@disable={{true}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell02
</tabs.Title>
<tabs.Title
@index={{3}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell03
</tabs.Title>
<tabs.Title
@index={{4}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell04
</tabs.Title>
</div>
<div class="mx-auto w-full max-w-md shadow-md px-4 py-4 rounded-xl bg-white mt-4">
<tabs.Content class="content">
<p>hello world</p>
</tabs.Content>
<tabs.Content class="content">
<p>hello world 2</p>
</tabs.Content>
<tabs.Content class="content">
<p>hello world 3</p>
</tabs.Content>
<tabs.Content class="content">
<p>hello world 4</p>
</tabs.Content>
</div>
</TabsGroup>
</div>`);
assert.dom('[data-test-ember-headlessui-tabs-wrapper]').exists();
});

test('Should render the respective content when a tab is selected', async function (assert) {
await render(hbs`<div class='w-full max-w-md px-2 py-16 mx-auto'>
<TabsGroup class="flex flex-col space-x-1 rounded-xl p-1" as |tabs|>
<div class='flex space-x-1 rounded-xl bg-sky-600/20 p-1'>
<tabs.Title
@index={{1}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'
>
hello
</tabs.Title>
<tabs.Title
@index={{2}}
@disable={{true}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell02
</tabs.Title>
<tabs.Title
@index={{3}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell03
</tabs.Title>
<tabs.Title
@index={{4}}
class='w-full rounded-lg py-2.5 text-sm font-medium leading-5 ring-white ring-opacity-60 ring-offset-2 ring-offset-blue-400 focus:outline-none focus:ring-2 space-x-4'>
hell04
</tabs.Title>
</div>
<div class="mx-auto w-full max-w-md shadow-md px-4 py-4 rounded-xl bg-white mt-4">
<tabs.Content class="content" @content-index={{1}}>
<p>hello world</p>
</tabs.Content>
<tabs.Content class="content" @content-index={{2}}>
<p>hello world 2</p>
</tabs.Content>
<tabs.Content class="content" @content-index={{3}}>
<p>hello world 3</p>
</tabs.Content>
<tabs.Content class="content" @content-index={{4}}>
<p>hello world 4</p>
</tabs.Content>
</div>
</TabsGroup>
</div>
`);
await click('[data-test-headlessui-tabs-title="test-tabs-title-btn-3"]');
assert
.dom('[data-test-headlessui-tabs-content="test-tabs-content-1"]')
.doesNotExist();
assert
.dom('[data-test-headlessui-tabs-content="test-tabs-content-2"]')
.doesNotExist();
assert
.dom('[data-test-headlessui-tabs-content="test-tabs-content-4"]')
.doesNotexist();
assert
.dom('[data-test-headlessui-tabs-content="test-tabs-content-3"]')
.exists();
});
});