clients
clients copied to clipboard
[EC 456] Component Library Content Switching Tabs
Type of change
- [ ] Bug fix
- [X] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
Add support for content switching tabs in addition to routing tabs. This implementation takes a lot of inspiration from the Angular Material tabs (source here).
In order to better support both content tabs and routing tabs (similar to Angular Material), the existing routing tab component was renamed to <bit-tab-nav-bar>
(with <bit-tab-link>
items) and the content switching tab component is now called <bit-tab-group>
(with <bit-tab>
items).
<bit-tab-nav-bar>
The underlying routing tab remains mostly unchanged, with some improvements to ARIA support (link
role, aria-disabled
, aria-current
) and keyboard navigation (arrow keys and spacebar, per spec).
Example usage:
<bit-tab-nav-bar label="Main">
<bit-tab-link [route]="['active']">Active</bit-tab-link>
<bit-tab-link [route]="['item-2']">Item 2</bit-tab-link>
<bit-tab-link [route]="['item-3']">Item 3</bit-tab-link>
<bit-tab-link [route]="['disable']" [disabled]="true">Disabled</bit-tab-link>
</bit-tab-nav-bar>
<bit-tab-group>
The content tab component supports dynamic tabs, label templates, and optional content preservation. ARIA support was also implemented, including tab
and tabpanel
roles, tabIndex
, aria-selected
, and aria-labeledby
attributes.
By default, each tab's content is only rendered to the DOM when the tab is selected (unless the preserveContent
flag is true
, in which case it is preserved).
Example usage:
<bit-tab-group label="Main Content Tabs">
<bit-tab label="First Tab">First Tab Content</bit-tab>
<bit-tab label="Second Tab">Second Tab Content</bit-tab>
<bit-tab>
<ng-template bit-tab-label>
<i class="bwi bwi-search tw-pr-1"></i> Template Label
</ng-template>
Template Label Content
</bit-tab>
<bit-tab [disabled]="true" label="Disabled">
Disabled Content
</bit-tab>
</bit-tab-group>
Code changes
Content Tabs Changes
-
libs/components/src/tabs/tab-group.component.ts: Update the
bit-tab-group
component to support content switching and template labels. -
libs/components/src/tabs/tab-item.component.html: Removed/replaced by the
bit-tab-list-item
directive. -
libs/components/src/tabs/tab-label.directive.ts: Directive used for identifying
<ng-template>
tags that will be used as the tab label instead of a plain text label. -
libs/components/src/tabs/tab.component.ts: Add the
bit-tab
component defining tabs and their respective content and labels. Supports both text and template labels for tab headers.
Routing Tabs Changes
-
libs/components/src/tabs/tab-link.component.html: Add tab nav link component to be used within the
bit-tab-nav-bar
component. Uses an underlying<a>
tag and[routerLink]
androuterLinkActive
directives. -
libs/components/src/tabs/tab-nav-bar.component.ts: Add the
bit-tab-nav-bar
component which handles rendering the tab links and keyboard navigation.
Shared Changes
-
libs/components/src/tabs/tabs.module.ts: Update the module to only export the necessary public components/directives. Keeps implementation specific directives internal to avoid polluting public API.
-
libs/components/src/tabs/tab-list-container.directive.ts: Internal helper directive used for styling the tab header items in both the content and router tab components.
-
libs/components/src/tabs/tab-list-item.directive.ts: Internal helper directive used for styling and interacting with tab header items.
-
libs/components/src/tabs/tabs.stories.ts: Update and add additional tab stories to storybook.
Screenshots
Content Tabs:
Route Tabs (content is only off center due to storybook story):
Before you submit
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team
Can we also create two sub-folders, one for nav items and another for content tabs? Will make it easier to separate them.
All in all good work! I especially like how well the accessibility features work! I've added some questions and suggestions and also have some general things I've noticed
- nit:
any
is used a few places, I thinkunknown
is generally preferred as a more type-safe alternative. Maybe you could do a quick search/replace?- issue: the tab header is missing a gray container/padding (see figma)
- question:
forwardRef
and your comment about circular dependencies makes me a bit uneasy. I've noticed that the CDK source has the same things. Have you given this any though? Do we know why? Does complexity increase a lot by trying to get rid of the circular dependencies?- suggestion: In the rest of the CL, directives seem to have
camelCase
selectors while custom tags (usually components) havekebab-case
. Just go through and tweak the namings a bit to make sure we follow the same convention
Thanks for the review and suggestions @coroiu!
- Good call, the couple uses of
any
are nowunknown
. - I added an additional wrapper to style the background gray and give that small spacing above the tabs.
- The circular dependency with the
forwardRef
seems mostly unavoidable here for the tab links and nav bar. The tab nav links need to communicate to the navbar when the active status changes (from the internalrouterLinkActive
directive) via theupdateActiveLink()
method. It's probably possible to keep track of the active route from the nav bar, but that would require subscribing to the active route and matching/checking against all the routes in the tab links which seems more cumbersome. At least when compared to using theforwardRef
to communicate between the components directly. But I'm open to re-evaluating this if you see any easier/better way. - Done, thanks for bringing that up, much more consistent now.
@shane-melton this looks good to me. Minor request, could you update the text-color
for the tab content stories so the tab's content uses text-main
? That way in dark mode the "First tab content" is more visible.
Sorry for dismissing my approval but I just noticed something in the figma design when it comes to the paddings
Here is the design for the tab component (which your PR mirrors very nicely)
And here is an example of the tabs being used in a dialog
As you can see the tabs have a slight padding that matches the content padding. I guess this could be added by a parent above tab component but I don't know if that's the best solution.
Also do we need to support both cases (with and w/o padidng), or if this is just a miss in figma?
do we need to support both cases (with and w/o padidng), or if this is just a miss in figma? Nope this was a miss in Figma; I'll update to include the padding.
Nice catch @coroiu! Padding is all fixed and matching the updated Figma now
@coroiu It was also decided with @danielleflinn to add content padding to the tabs and adjust the bottom border of tab header to span the entire content area (to match the examples in tabbed dialogs). We'll still need another PR for the dialog component to optionally remove its content padding (e.g. when the content is a tab group) otherwise we'll still get double padding in dialogs.