fix: set DBTabItem selected state correctly

.changeset/sharp-pumas-obey.md

@@ -0,0 +1,6 @@
+---
+"@db-ux/core-components": patch
+---
+
+fix: set DBTabItem internal state `_selected` correctly
+- Now also sets aria-selected=true|false correctly which improves screen reader behaviour

packages/components/src/components/tab-item/model.ts

@@ -49,6 +49,9 @@ export type DBTabItemProps = GlobalProps &
 
 export type DBTabItemDefaultState = {
 	_selected: boolean;
+	_listenerAdded: boolean;
+	boundSetSelectedOnChange?: (event: any) => void;
+	setSelectedOnChange: (event: any) => void;
 };
 
 export type DBTabItemState = DBTabItemDefaultState &

packages/components/src/components/tab-item/tab-item.lite.tsx

@@ -1,5 +1,6 @@
 import {
 	onMount,
+	onUnMount,
 	onUpdate,
 	Show,
 	useDefaultProps,
@@ -27,11 +28,25 @@ useDefaultProps<DBTabItemProps>({});
 
 export default function DBTabItem(props: DBTabItemProps) {
 	const _ref = useRef<HTMLInputElement | any>(null);
+
 	// jscpd:ignore-start
 	const state = useStore<DBTabItemState>({
 		_selected: false,
 		_name: undefined,
 		initialized: false,
+		_listenerAdded: false,
+		boundSetSelectedOnChange: undefined,
+		setSelectedOnChange: (event: any) => {
+			event.stopPropagation();
+			useTarget({
+				stencil: () => {
+					state._selected = getBooleanAsString(event.target === _ref);
+				},
+				default: () => {
+					state._selected = event.target === _ref;
+				}
+			});
+		},
 		handleNameAttribute: () => {
 			if (_ref) {
 				const setAttribute = _ref.setAttribute;
@@ -44,23 +59,10 @@ export default function DBTabItem(props: DBTabItemProps) {
 			}
 		},
 		handleChange: (event: any) => {
-			event.stopPropagation();
 			if (props.onChange) {
 				props.onChange(event);
 			}
 
-			// We have different ts types in different frameworks, so we need to use any here
-
-			useTarget({
-				stencil: () => {
-					const selected = (event.target as any)?.['checked'];
-					state._selected = getBooleanAsString(selected);
-				},
-				default: () => {
-					state._selected = (event.target as any)?.['checked'];
-				}
-			});
-
 			useTarget({
 				angular: () =>
 					handleFrameworkEventAngular(state, event, 'checked'),
@@ -69,28 +71,73 @@ export default function DBTabItem(props: DBTabItemProps) {
 		}
 	});
 
+	// Set up event listener to react on any change (select & deselect) in tab list
+	// Default: Most framework can just pass the state function to the parents event listener.
+	// Stencil: Bind the function to maintain correct 'this' context in class components
+	// React: Wrap in arrow function so setState doesn't treat it as a state updater
 	onMount(() => {
+		useTarget({
+			stencil: () => {
+				state.boundSetSelectedOnChange =
+					state.setSelectedOnChange.bind(state);
+			},
+			react: () => {
+				state.boundSetSelectedOnChange = () =>
+					state.setSelectedOnChange;
+			},
+			default: () => {
+				state.boundSetSelectedOnChange = state.setSelectedOnChange;
+			}
+		});
 		state.initialized = true;
 	});
 	// jscpd:ignore-end
 
 	onUpdate(() => {
-		if (state.initialized && _ref) {
-			if (props.active) {
-				_ref.click();
-			}
-
+		if (_ref && state.initialized && state.boundSetSelectedOnChange) {
 			useTarget({ react: () => state.handleNameAttribute() });
 			state.initialized = false;
+
+			// deselect this tab when another tab in tablist is selected
+			if (!state._listenerAdded) {
+				_ref.closest('[role=tablist]')?.addEventListener(
+					'change',
+					state.boundSetSelectedOnChange
+				);
+				state._listenerAdded = true;
+			}
+
+			// Initialize selected state from either active prop (set by parent) or checked attribute
+			if (props.active || _ref.checked) {
+				useTarget({
+					stencil: () => {
+						state._selected = getBooleanAsString(true);
+					},
+					default: () => {
+						state._selected = true;
+					}
+				});
+				_ref.click();
+			}
 		}
-	}, [_ref, state.initialized]);
+	}, [_ref, state.initialized, state.boundSetSelectedOnChange]);
 
 	onUpdate(() => {
 		if (props.name) {
 			state._name = props.name;
 		}
 	}, [props.name]);
 
+	onUnMount(() => {
+		if (state._listenerAdded && _ref && state.boundSetSelectedOnChange) {
+			_ref.closest('[role=tablist]')?.removeEventListener(
+				'change',
+				state.boundSetSelectedOnChange
+			);
+			state._listenerAdded = false;
+		}
+	});
+
 	return (
 		<li class={cls('db-tab-item', props.className)} role="none">
 			<label

packages/components/src/components/tabs/tabs.lite.tsx

@@ -170,11 +170,11 @@ export default function DBTabs(props: DBTabsProps) {
 					if (tabItem) {
 						const list = tabItem.parentElement;
 						if (list) {
-							const indices = Array.from(list.childNodes).indexOf(
+							const tabIndex = Array.from(list.children).indexOf(
 								tabItem
 							);
 							if (props.onIndexChange) {
-								props.onIndexChange(indices);
+								props.onIndexChange(tabIndex);
 							}
 
 							if (props.onTabSelect) {

packages/components/src/components/tabs/tabs.spec.tsx

@@ -1,34 +1,34 @@
 import AxeBuilder from '@axe-core/playwright';
 import { expect, test } from '@playwright/experimental-ct-react';
 
-import { existsSync, rmSync, writeFileSync } from 'node:fs';
 import { DBTabs } from './index';
 // @ts-ignore - vue can only find it with .ts as file ending
 import { DEFAULT_VIEWPORT } from '../../shared/constants.ts';
 import { DBTabItem } from '../tab-item';
 import { DBTabList } from '../tab-list';
 import { DBTabPanel } from '../tab-panel';
 
-const filePath = './test-results/onIndexChange.txt';
+let activeTabIndex: number | undefined;
+let comp: any = null;
 
-const comp: any = (
-	<DBTabs
-		onIndexChange={(index: number) =>
-			writeFileSync(filePath, index.toString())
-		}>
-		<DBTabList>
-			<DBTabItem data-testid="test">Test 1</DBTabItem>
-			<DBTabItem data-testid="test2">Test 2</DBTabItem>
-			<DBTabItem>Test 3</DBTabItem>
-		</DBTabList>
+test.beforeEach(() => {
+	activeTabIndex = undefined;
+	comp = (
+		<DBTabs onIndexChange={(index: number) => (activeTabIndex = index)}>
+			<DBTabList>
+				<DBTabItem data-testid="test">Test 1</DBTabItem>
+				<DBTabItem data-testid="test2">Test 2</DBTabItem>
+				<DBTabItem>Test 3</DBTabItem>
+			</DBTabList>
 
-		<DBTabPanel>TestPanel 1</DBTabPanel>
+			<DBTabPanel>TestPanel 1</DBTabPanel>
 
-		<DBTabPanel>TestPanel 2</DBTabPanel>
+			<DBTabPanel>TestPanel 2</DBTabPanel>
 
-		<DBTabPanel>TestPanel 3</DBTabPanel>
-	</DBTabs>
-);
+			<DBTabPanel>TestPanel 3</DBTabPanel>
+		</DBTabs>
+	);
+});
 
 const testComponent = () => {
 	test('should contain text', async ({ mount }) => {
@@ -44,10 +44,11 @@ const testComponent = () => {
 
 const testActions = () => {
 	test('should be clickable', async ({ mount }) => {
-		if (existsSync(filePath)) {
-			rmSync(filePath);
-		}
+		expect(activeTabIndex).toBe(undefined);
 
+		// Beware: the comments below actually change the selector for vue
+		// this is necessary because vue will not trigger a check on an list element but requires the actual
+		// radio button element, which has the role=tab
 		const component = await mount(comp);
 		await component
 			.getByTestId('test2')
@@ -59,7 +60,7 @@ const testActions = () => {
 			.isChecked();
 		expect(!tabChecked).toBeTruthy();
 
-		expect(existsSync(filePath)).toBeTruthy();
+		expect(activeTabIndex).toBe(1);
 	});
 };