feat(cdk-experimental/ui-patterns): tabs ui pattern #30568

ok7sai · 2025-02-28T18:20:13Z

No description provided.

wagnermaciel · 2025-02-28T20:10:10Z

src/cdk-experimental/tabs/tabs.ts

+    '[attr.tabindex]': 'pattern.tabindex()',
+    '[attr.aria-disabled]': 'pattern.disabled()',
+    '[attr.aria-orientation]': 'pattern.orientation()',
+    '[attr.aria-multiselectable]': 'pattern.multiselectable()',


Could you share a link to the reference for multiselectable tabs? My suspicion is that even in situations where tabs are multiselectable, only one tab's content is displayed

https://w3c.github.io/aria/#tablist specified aria-multiselectable is a supported attribute for tablist role, but so far I can't find a working example of a multi-selectable tabs.

However while searching around I found this w3c/aria#1355, so it is unclear at this point what a multi-selectable tabs will look like.

Based on the above issue and per our previous discussion, I removed the multi-selectable part from the tabs ui pattern.

I think the pattern exists. I imagine it works similar to how multiselecting tabs in the browser works. We should document this as a potential feature we want to add in the future

Sounds good. I will include in the tabs pattern design doc.

IMO we should exclude this from a the first iteration and consider as a possible feature addition later.

src/cdk-experimental/ui-patterns/tabs/tablist.ts

src/components-examples/cdk-experimental/tabs/cdk-tabs/cdk-tabs-example.css

src/cdk-experimental/ui-patterns/tabs/tablist.ts

src/cdk-experimental/tabs/tabs.ts

src/cdk-experimental/ui-patterns/tabs/tabpanel.ts

crisbeto · 2025-03-03T08:26:09Z

src/cdk-experimental/tabs/tabs.ts

+})
+export class CdkTabs {
+  /** The CdkTabs nested inside of the container. */
+  private readonly _cdkTabs = contentChildren(CdkTab, {descendants: true});


Should we start without having descendants: true? The problem is that this will also pick up tabs and panels inside of nested tabs, which apparently is somewhat common.

You are right, we shouldn't include the nested tabs/tabpanels, those are supposed to be controlled by another CdkTabs directive. Fixed.

crisbeto · 2025-03-03T08:26:49Z

src/cdk-experimental/tabs/tabs.ts

+    '[attr.aria-orientation]': 'pattern.orientation()',
+    '[attr.aria-activedescendant]': 'pattern.activedescendant()',
+    '(keydown)': 'pattern.onKeydown($event)',
+    '(mousedown)': 'pattern.onPointerdown($event)',


Should we use pointer events here?

Oops fixed.

crisbeto · 2025-03-03T08:29:08Z

src/cdk-experimental/tabs/tabs.ts

+    'role': 'tabpanel',
+    'class': 'cdk-tabpanel',
+    '[attr.id]': 'pattern.id()',
+    '[attr.aria-hidden]': 'pattern.hidden()',


Should we use inert instead of aria-hidden? The former will also block focus and keyboard events from entering the hidden content.

I think that's a good idea and inert makes it removed from the a11y tree. What I am not sure is how much customization we want to leave to developers.

aria-hidden: developers handle both visualization and interaction.

inert: developers handle visualization.

I don't think there's a use case where the off-screen tabs shouldn't be inert so I think we can start with that and see if users ask for more customization.

Sounds good. Changed to inert instead.

src/cdk-experimental/ui-patterns/tabs/tablist.ts

crisbeto · 2025-03-03T08:32:11Z

src/cdk-experimental/ui-patterns/tabs/tablist.ts

+  }
+
+  /** Navigates to the first option in the tablist. */
+  first(opts?: SelectOptions) {


If we have the goto method, do we need first, last, next and prev?

This part is mostly followed the implementation in https://github.com/angular/components/blob/ef62d5c8e37bb7cc0b7ca9f8c5662593516f295f/src/cdk-experimental/ui-patterns/listbox/listbox.ts for consistency.

I find the current design very readable and each navigation method is thin and light. With only goto method, I guess we will need an input for direction?

Let's leave it for now, but I think we should revisit it both for tabs and list.

These are really convenience methods, but they also handle subtle complexity around skipping disabled items and wrapping

src/cdk-experimental/ui-patterns/tabs/tablist.ts

jelbourn · 2025-03-05T00:21:54Z

src/cdk-experimental/tabs/tabs.ts

+    'class': 'cdk-tabs',
+  },
+})
+export class CdkTabs {


I'm a little unsure about having CdkTabs as a necessary wrapper / container vs. the tabpanel having a reference/input for the tablist, implying a 1:1 relationship between tablist and tabpanel.

I was going to include the decision making of choosing a top-level wrapper in the design doc for later. Here's a quick comparison.

Passing references

<ul cdkTablist> <li cdkTab #tab1="tab">tab1</li> <li cdkTab #tab2="tab">tab2</li> </ul> <div cdkTabpanel [for]="tab1"></div> <div cdkTabpanel [for]="tab2"></div>

Note that the template variable referencing can be reversed. The only dealbreaker I can think of is the dynamic generated tabs or tabpanels via control flow may be impossible. Another downside is the templating effort of creating template references for general use case.

The wrapper solution on the other hand can support both implicit and explicit(yet implemented, but likely via a string identifier other than id) tab-tabpanel binding, which I think it's more intuitive. I also found the same pattern used in most similar libraries, so it can be familiar to developers as well.

Yeah, I think we should probably have a discussion on the API(s) we expect an end-developer to define for their own tab implementations, and then how to best accommodate that with the directives. FWIW I think we will have a requirement that there may be elements between the tablist and the tabpanel(s).

For content, I suspect these directives will need to end up being oriented around rendering <ng-template> content.

jelbourn · 2025-03-05T00:23:01Z

src/cdk-experimental/tabs/tabs.ts

+  skipDisabled = input(true, {transform: booleanAttribute});
+
+  /** The focus strategy used by the tablist. */
+  focusMode = input<'roving' | 'activedescendant'>('roving');


I don't think we should support activedescendant for tabs; I'm not aware of any case where you'd want that instead of roving tabindex

Agreed it's probably not common. Although I did find a combobox+tabs example https://ariakit.org/examples/combobox-tabs

Oh wow, that's wild. I guess there's no real harm if roving tabindex if the default

jelbourn · 2025-03-05T00:23:43Z

src/cdk-experimental/tabs/tabs.ts

+  disabled = input(false, {transform: booleanAttribute});
+
+  /** The ids of the current selected tab. */
+  selectedIds = model<string[]>([]);


I think we should only support a single selected tab in this initial version.

sounds good. removed

jelbourn · 2025-03-05T00:24:28Z

src/cdk-experimental/tabs/tabs.ts

+  });
+
+  /** The Tab UIPatterns of the child Tabs. */
+  protected items = computed(() => this._cdkTabs().map(tab => tab.pattern));


Suggested change

protected items = computed(() => this._cdkTabs().map(tab => tab.pattern));

protected tabs = computed(() => this._cdkTabs().map(tab => tab.pattern));

Call this tabs?

src/cdk-experimental/tabs/tabs.ts

jelbourn · 2025-03-05T00:32:50Z

src/cdk-experimental/ui-patterns/tabs/tab.ts

+  id: Signal<string>;
+
+  /** The position of the tab in the list. */
+  index = computed(


Should index be on the public API of TabPattern? It feels like perhaps an implementation detail leaking through

Removed. It's actually unused. I added the index to cdk and forgot to clean this up. :p

jelbourn · 2025-03-05T00:34:50Z

src/cdk-experimental/ui-patterns/tabs/tab.ts

+/** The required inputs to tabs. */
+export interface TabInputs extends ListNavigationItem, ListSelectionItem, ListFocusItem {
+  tablist: Signal<TablistPattern>;
+  tabpanel: Signal<TabpanelPattern>;


It seems odd to me that the tab would need to know about the tabpanel. I was imagining the relationship would be that the tabpanel only knows about the value of the tablist, not necessarily any of the specifics tabs

jelbourn · 2025-03-05T00:35:40Z

src/cdk-experimental/ui-patterns/tabs/tab.ts

+import {ListFocus, ListFocusItem} from '../behaviors/list-focus/list-focus';
+import {TabpanelPattern} from './tabpanel';
+
+interface TablistPattern {


This interface is not connected to from the real implementation of TablistPattern

oops that's right. fixed.

jelbourn · 2025-03-05T00:38:13Z

src/cdk-experimental/ui-patterns/tabs/tabpanel.ts

+  /** Whether the tabpanel is hidden. */
+  hidden = computed(() => !this.tab().selected());
+
+  constructor(args: TabpanelInputs) {


Suggested change

constructor(args: TabpanelInputs) {

constructor(inputs: TabpanelInputs) {

jelbourn · 2025-03-06T23:43:55Z

src/cdk-experimental/tabs/tabs.ts

+    'role': 'tabpanel',
+    'class': 'cdk-tabpanel',
+    '[attr.id]': 'pattern.id()',
+    '[attr.inert]': 'pattern.hidden() ? true : null',


Add a comment explaining the purpose of the inert attribute in this context? Any links for what this is based on? (I haven't seen this approach before)

Added comment to the JSDoc for the CdkTabpanel.

jelbourn · 2025-03-06T23:44:35Z

src/cdk-experimental/tabs/tabs.ts

+  private readonly _cdkTablist = inject(CdkTablist);
+
+  /** A unique identifier for the tab. */
+  private readonly _generatedId = inject(_IdGenerator).getId('cdk-tab-');


Suggested change

private readonly _generatedId = inject(_IdGenerator).getId('cdk-tab-');

private readonly _id = inject(_IdGenerator).getId('cdk-tab-');

Based on #30597, you should be able to drop the unnecessary computed and just pass () => this._id to satisfy the type for the ID to the pattern

(and similar for the other unnecessary computeds)

Done, and also replaced some unnecessary computeds with pure functions.

jelbourn · 2025-03-06T23:51:23Z

src/cdk-experimental/ui-patterns/tabs/tabpanel.ts

+}
+
+/** A tabpanel associated with a tab. */
+export class TabpanelPattern {


This is probably a bigger conversation, but I wonder whether it makes sense to have a tabpanel pattern at all, since it doesn't actually do or render anything.

jelbourn · 2025-03-07T00:31:19Z

src/cdk-experimental/tabs/tabs.ts

+    'class': 'cdk-tabs',
+  },
+})
+export class CdkTabs {


Yeah, I think we should probably have a discussion on the API(s) we expect an end-developer to define for their own tab implementations, and then how to best accommodate that with the directives. FWIW I think we will have a requirement that there may be elements between the tablist and the tabpanel(s).

For content, I suspect these directives will need to end up being oriented around rendering <ng-template> content.

jelbourn · 2025-03-07T00:32:32Z

src/cdk-experimental/tabs/tabs.ts

+ *   <div cdkTabpanel>Tab content 1</div>
+ *   <div cdkTabpanel>Tab content 2</div>
+ *   <div cdkTabpanel>Tab content 3</div>


Do we have any info on whether having one tabpanel element for each tab is strictly necessary, or can there be one tabpanel that changes attributes/content?

cc @crisbeto I vaguely recall this has come up before but I don't remember the details. Do you happen to recall anything?

Hidden tabpanels should have at least aria-hidden attribute, from the accessibility tree perspective there will be only one tabpanel, so having one or one per tab are likely the same. Most of the ARIA examples have one tabpanel per tab due to the static content.

It's probably fine supporting both ways

 <div cdkTabs> <ul cdkTablist> <li cdkTab>tab 1</li> <li cdkTab>tab 2</li> </ul> <div cdkTabpanel>content 1</div> <div cdkTabpanel>content 2</div>  @for (tabpanel of tabpanels) { <div cdkTabpanel>{{tabpanel.content}}</div> } </div>  <div cdkTabs> <ul cdkTablist #tablist> <li cdkTab tab="tab1">tab 1</li> <li cdkTab tab="tab2">tab 2</li> </ul>  <div cdkTabpanel [tab]="tablist.selected...">{{getContent(...)}}</div> </div>

explaining the `inert` attribute

ok7sai requested a review from a team as a code owner February 28, 2025 18:20

ok7sai requested review from crisbeto and andrewseguin and removed request for a team February 28, 2025 18:20

angular-robot bot added the detected: feature PR contains a feature commit label Feb 28, 2025

wagnermaciel reviewed Feb 28, 2025

View reviewed changes

ok7sai force-pushed the ng-aria-tabs branch from 9fbf887 to 06dd309 Compare February 28, 2025 23:01

crisbeto reviewed Mar 3, 2025

View reviewed changes

ok7sai force-pushed the ng-aria-tabs branch 2 times, most recently from f82703c to eed8a03 Compare March 4, 2025 18:04

wagnermaciel requested a review from jelbourn March 4, 2025 23:43

wagnermaciel approved these changes Mar 5, 2025

View reviewed changes

jelbourn reviewed Mar 5, 2025

View reviewed changes

ok7sai force-pushed the ng-aria-tabs branch from 1181b5b to 14c3560 Compare March 6, 2025 17:48

jelbourn reviewed Mar 7, 2025

View reviewed changes

ok7sai added 10 commits March 7, 2025 01:18

feat(cdk-experimental/ui-patterns): tabs ui pattern

e8b1f97

Remove multiselectable part from tabs ui pattern

0af0bf7

Add JSDoc to cdk tabs directives

22d8949

Addressed comments

bf3a806

Replaced aria-hidden with inert

3483c31

Addressed comments

0931bf6

Removed selectedIds from CdkTablist

2224232

Removed unused index from tab ui pattern

8318ea2

Moved from Signal type to SignalLike type

f783e3d

Replaced unnecessary computes with pure functions, and added comment

c87bd4c

explaining the `inert` attribute

ok7sai force-pushed the ng-aria-tabs branch from 14c3560 to c87bd4c Compare March 7, 2025 01:18

	protected items = computed(() => this._cdkTabs().map(tab => tab.pattern));
	protected tabs = computed(() => this._cdkTabs().map(tab => tab.pattern));

	constructor(args: TabpanelInputs) {
	constructor(inputs: TabpanelInputs) {

	private readonly _generatedId = inject(_IdGenerator).getId('cdk-tab-');
	private readonly _id = inject(_IdGenerator).getId('cdk-tab-');

feat(cdk-experimental/ui-patterns): tabs ui pattern #30568

Are you sure you want to change the base?

feat(cdk-experimental/ui-patterns): tabs ui pattern #30568

Conversation

ok7sai commented Feb 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ok7sai Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ok7sai Mar 5, 2025 •

edited

Loading