Skip to content

fix(core): stabilize flaky component tests - #2666

Closed
danielleroux wants to merge 14 commits into
mainfrom
fix/misc-fixing-flaky-components
Closed

danielleroux wants to merge 14 commits into
mainfrom
fix/misc-fixing-flaky-components

Conversation

@danielleroux

@danielleroux danielleroux commented Jul 21, 2026 •

Copy link
Copy Markdown
Collaborator

🆕 What is the new behavior?

  • stabilize flaky component tests

🏁 Checklist

A pull request can only be merged if all of these conditions are met (where applicable):

  • 🦮 Accessibility (a11y) features were implemented
  • 🗺️ Internationalization (i18n) - no hard coded strings
  • 📲 Responsiveness - components handle viewport changes and content overflow gracefully
  • 📕 Add or update a Storybook story
  • 📄 Documentation was reviewed/updated siemens/ix-docs
  • 🧪 Unit tests were added/updated and pass (pnpm test)
  • 📸 Visual regression tests were added/updated and pass (Guide)
  • 🧐 Static code analysis passes (pnpm lint)
  • 🏗️ Successful compilation (pnpm build, changes pushed)

Related

  • EIX-111

👨‍💻 Help & support

Summary by CodeRabbit

Bug Fixes

  • Menus now remain expanded after item selection when pinned.
  • Dropdowns open more reliably after rendering and when triggers are added dynamically.
  • Date picker keyboard navigation keeps focus on the expected day and prevents unintended scrolling.
  • Tabs and About panels activate reliably while content initializes.

Accessibility

  • Improved ARIA forwarding and normalization, including labels and toggle state attributes.

Tests

  • Expanded regression and accessibility coverage for menus, dropdowns, tabs, and form controls.

@netlify

netlify Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for ix-storybook ready!

Name Link
🔨 Latest commit f984fb5
🔍 Latest deploy log https://app.netlify.com/projects/ix-storybook/deploys/6ad0d976009c16b52f34c733
😎 Deploy Preview https://deploy-preview-2666--ix-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@changeset-bot

changeset-bot Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f984fb5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@siemens/ix Patch
@siemens/ix-angular Patch
@siemens/ix-react Patch
@siemens/ix-vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Updates dropdown trigger initialization, tab and menu synchronization, date-picker focus handling, ARIA forwarding, and regression tests. Patch changesets document the behavioral fixes for @siemens/ix.

Changes: Component interaction and accessibility fixes

Layer / File(s) Summary
Dropdown trigger resolution and interaction flow
packages/core/src/components/dropdown/..., packages/core/src/components/dropdown/test/..., .changeset/steady-dropdown-triggers.md, .changeset/mighty-poems-behave.md
Dropdowns resolve immediate and dynamic triggers, match click paths, ignore stale asynchronous results, and await trigger registration. Tests wait for hydrated trigger state.
Tab, menu, and date-picker state synchronization
packages/core/src/components/tab-set/tab-set.tsx, packages/core/src/components/tabs/..., packages/core/src/components/menu-about/menu-about.tsx, packages/core/src/components/menu/menu.tsx, packages/core/src/components/date-picker/date-picker.tsx, .changeset/*
Tab keys resolve from properties or attributes. Panel synchronization is scheduled. Legacy tab activation and menu expansion behavior are updated. Date-picker navigation synchronizes focus targets and prevents browser scrolling.
ARIA forwarding and managed accessibility state
packages/core/src/components/utils/..., packages/core/src/components/toggle/toggle.tsx, packages/core/src/components/{chip,icon-button,popover,tabs,toggle}/test*, .changeset/fresh-buttons-rest.md
ARIA removal interception and reflection handling are added. Inherited attributes track forwarded removals. Toggle-managed ARIA state is restored and tested.
Regression synchronization and assertions
packages/core/src/components/{chat-input,checkbox,input,menu-category,menu,split-button,tree,datetime-input,time-picker}/test*
Tests add hydration and rendering-settle waits and update overflow, accessibility, keyboard, layout, form-value, active-descendant, focus, and time-selection assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant DropdownController
  participant Dropdown
  User->>DropdownController: Click trigger path
  DropdownController->>Dropdown: Match event targets
  DropdownController->>Dropdown: Toggle dropdown and dismiss others
  Dropdown->>Dropdown: Resolve trigger and register listeners
Loading

Possibly related PRs

  • siemens/ix#2509: Related dropdown trigger and input initialization race-condition fixes.
  • siemens/ix#2574: Related dropdown-controller trigger-path and dismissal handling.
  • siemens/ix#2659: Related dropdown trigger resolution and controller matching changes.

Suggested reviewers: nuke-ellington

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the pull request’s main goal of stabilizing flaky core component tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/misc-fixing-flaky-components

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several bug fixes and stability improvements across components like ix-menu, ix-tab-set, ix-date-picker, and ix-dropdown, along with corresponding updates to component tests. The review feedback highlights two important issues: first, changing !this.showPinned to !this.pinned in menu.tsx introduces a regression on desktop where the menu collapses unexpectedly upon item selection; second, the matchesTrigger implementation in dropdown.tsx should restrict ID matching to the same shadow root or document scope to prevent selector conflicts between multiple web components.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/core/src/components/dropdown/dropdown.tsx (1)

745-757: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Missing cleanup when trigger becomes falsy.

If the trigger property is unset or changed to a falsy value (e.g., undefined), the condition newTriggerValue && ... short-circuits. This prevents disposeClickListener() from executing, leaving the dropdown permanently bound to the old, removed trigger.

Change the condition to dispose of the old listeners as long as the value has changed.

🐛 Proposed fix
  `@Watch`('trigger')
  async changedTrigger(
    newTriggerValue: ElementReference,
    oldTriggerValue: ElementReference | undefined
  ) {
-   if (newTriggerValue && newTriggerValue !== oldTriggerValue) {
+   if (newTriggerValue !== oldTriggerValue) {
      this.disposeClickListener?.();
      this.disposeClickListener = undefined;
      this.disposeKeyListener?.();
      this.disposeKeyListener = undefined;
    }

    await this.registerListener(newTriggerValue);
  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/components/dropdown/dropdown.tsx` around lines 745 - 757,
Update changedTrigger so listener cleanup runs whenever newTriggerValue differs
from oldTriggerValue, including when the new value is falsy. Preserve the
existing disposal and reset behavior for both disposeClickListener and
disposeKeyListener, then continue registering the new trigger.
packages/core/src/components/tab-set/tab-set.tsx (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared tabKey/tab-key fallback into a common utility. Both tab-set.tsx and menu-about.tsx independently implement the identical prop ?? attribute ?? undefined fallback for resolving a tab key.

  • packages/core/src/components/tab-set/tab-set.tsx#L49-53: keep getTabKey() here, but extract its body into a shared utility function.
  • packages/core/src/components/menu-about/menu-about.tsx#L103-105: replace the inline fallback with the shared utility instead of re-implementing it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/components/tab-set/tab-set.tsx` at line 1, Extract the
shared tab-key resolution logic from getTabKey() in tab-set.tsx into a reusable
utility that returns the prop value, then the tab-key attribute, then undefined.
Update menu-about.tsx to call this utility instead of its inline fallback, while
keeping getTabKey() as the local wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/components/date-picker/date-picker.tsx`:
- Line 859: Update the parse call in the focused-day handling code to use
Number.parseInt instead of the global parseInt, preserving the radix argument
and existing assignment to focusedDay.

In `@packages/core/src/components/dropdown/dropdown.tsx`:
- Around line 319-331: Update matchesTrigger so the ID comparison only runs when
trigger is a non-empty string; preserve direct target matching and existing
behavior for valid trigger IDs.
- Around line 521-537: Update the trigger-resolution flow around
resolveImmediateElement to return immediately when element is falsy, preventing
resolution of an unset trigger. After the awaited element resolution, verify
that the resolved request still corresponds to the current trigger before
assigning triggerElement or attaching listeners, and discard stale results to
prevent orphaned listeners.

In `@packages/core/src/components/menu-about/menu-about.tsx`:
- Around line 102-106: Remove the duplicated tab key resolution logic from the
active-tab initialization in the menu-about component and reuse the shared
helper or implementation already defined in tab-set.tsx. Preserve the existing
behavior of preferring the item’s tabKey, then its tab-key attribute, when
assigning activeTabKey.

In `@packages/core/src/components/tab-set/tab-set.tsx`:
- Around line 49-53: Remove the duplicated tab-key fallback from TabSet’s
getTabKey and reuse the shared key-resolution helper already used by
menu-about.tsx. Preserve the existing precedence of the tabKey property, then
the tab-key attribute, with undefined when neither is available.

In `@packages/core/src/components/tree/test/tree.ct.ts`:
- Around line 542-551: Extract the nested requestAnimationFrame promise in
waitForTreeToSettle into a reusable nextFrame() helper, then await the helper
twice to preserve the existing two-frame settling behavior while reducing
nesting depth.

---

Outside diff comments:
In `@packages/core/src/components/dropdown/dropdown.tsx`:
- Around line 745-757: Update changedTrigger so listener cleanup runs whenever
newTriggerValue differs from oldTriggerValue, including when the new value is
falsy. Preserve the existing disposal and reset behavior for both
disposeClickListener and disposeKeyListener, then continue registering the new
trigger.

In `@packages/core/src/components/tab-set/tab-set.tsx`:
- Line 1: Extract the shared tab-key resolution logic from getTabKey() in
tab-set.tsx into a reusable utility that returns the prop value, then the
tab-key attribute, then undefined. Update menu-about.tsx to call this utility
instead of its inline fallback, while keeping getTabKey() as the local wrapper.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 86029ff2-4cd9-405c-8af8-7e8d87316a06

📥 Commits

Reviewing files that changed from the base of the PR and between 70e1c5c and febf143.

📒 Files selected for processing (20)
  • .changeset/persistent-pinned-menu.md
  • .changeset/reliable-tab-activation.md
  • .changeset/stable-date-picker-focus.md
  • .changeset/steady-dropdown-triggers.md
  • packages/core/src/components/chat-input/tests/chat-input.ct.ts
  • packages/core/src/components/checkbox/tests/checkbox.ct.ts
  • packages/core/src/components/date-picker/date-picker.tsx
  • packages/core/src/components/dropdown/dropdown-controller.ts
  • packages/core/src/components/dropdown/dropdown.tsx
  • packages/core/src/components/dropdown/test/dropdown-top-layer.ct.ts
  • packages/core/src/components/dropdown/test/dropdown.ct.ts
  • packages/core/src/components/input/tests/password-input.ct.ts
  • packages/core/src/components/menu-about/menu-about.tsx
  • packages/core/src/components/menu-about/test/menu-about.ct.ts
  • packages/core/src/components/menu-category/test/menu-category.ct.ts
  • packages/core/src/components/menu/menu.tsx
  • packages/core/src/components/split-button/test/split-button.ct.ts
  • packages/core/src/components/tab-set/tab-set.tsx
  • packages/core/src/components/tabs/test/tabs.ct.ts
  • packages/core/src/components/tree/test/tree.ct.ts
💤 Files with no reviewable changes (1)
  • packages/core/src/components/chat-input/tests/chat-input.ct.ts

@nuke-ellington nuke-ellington added this to the 5.2.0 milestone Jul 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/components/menu-category/test/menu-category.ct.ts`:
- Around line 438-449: Add an accessibility regression test to the menu-category
component test suite using the existing makeAxeBuilder() pattern. Run it against
the rendered menu-category interaction state and assert that the accessibility
scan reports no violations, reusing the file’s established setup and test
conventions.

In `@packages/core/src/components/menu/menu.tsx`:
- Around line 678-681: Add an axe-based component test for the standalone menu
close path in the menu test suite, exercising the branch in the close logic
where this.applicationLayoutContext is absent. Keep existing pinned/non-pinned
coverage intact and verify the standalone menu renders or closes with no
accessibility violations.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f2aaa58f-cdf3-425e-97a0-e11a008c4208

📥 Commits

Reviewing files that changed from the base of the PR and between 30f1c7c and 0d1949f.

📒 Files selected for processing (5)
  • packages/core/src/components/datetime-input/test/datetime-input.ct.ts
  • packages/core/src/components/dropdown/test/dropdown.ct.ts
  • packages/core/src/components/menu-category/test/menu-category.ct.ts
  • packages/core/src/components/menu/menu.tsx
  • packages/core/src/components/time-picker/test/time-picker.ct.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/core/src/components/tabs/test/tabs.ct.ts (1)

60-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not suppress the nested-interactive assertion without handling the root cause.

The closable-tab test only passes because .disableRules(['nested-interactive']) turns off coverage for that rule; the test still asserts violations is empty. Keep the accessibility scan enabled and avoid the nested interactive structure, or use a narrowly scoped, documented exception if the pattern is intentional.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/components/tabs/test/tabs.ct.ts` around lines 60 - 62,
Remove the disableRules(['nested-interactive']) suppression from the
accessibility scan in the closable-tab test and address the underlying nested
interactive markup so the scan remains fully enabled and violations stays empty;
only retain a narrowly scoped documented exception if that structure is
intentional.

Source: Path instructions

packages/core/src/components/toggle/test/toggle.ct.ts (1)

1-200: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Add an axe-based accessibility test for ix-toggle.

The file has a hydration/render test but no makeAxeBuilder accessibility check, despite this PR reworking role/aria-checked/aria-disabled/aria-required management. As per path instructions, "Ensure accessibility coverage exists with axe-based component tests where behavior/UI changed" and CT files should "include ... accessibility test using makeAxeBuilder()".

♿ Suggested addition
regressionTest('passes axe', async ({ mount, page, makeAxeBuilder }) => {
  await mount(`<ix-toggle aria-label="Notifications"></ix-toggle>`);
  const results = await makeAxeBuilder().analyze();
  expect(results.violations).toEqual([]);
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/components/toggle/test/toggle.ct.ts` around lines 1 - 200,
Add an axe-based accessibility regression test near the existing render tests,
using the regressionTest fixture’s makeAxeBuilder with an accessible ix-toggle
mount and asserting that analyze() returns no violations. Reuse the existing
expect import and keep the test focused on accessibility coverage.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.changeset/fresh-buttons-rest.md:
- Line 5: Update the public changelog text in the changeset to hyphenate
“component-managed attributes,” leaving the surrounding description unchanged.

---

Outside diff comments:
In `@packages/core/src/components/tabs/test/tabs.ct.ts`:
- Around line 60-62: Remove the disableRules(['nested-interactive']) suppression
from the accessibility scan in the closable-tab test and address the underlying
nested interactive markup so the scan remains fully enabled and violations stays
empty; only retain a narrowly scoped documented exception if that structure is
intentional.

In `@packages/core/src/components/toggle/test/toggle.ct.ts`:
- Around line 1-200: Add an axe-based accessibility regression test near the
existing render tests, using the regressionTest fixture’s makeAxeBuilder with an
accessible ix-toggle mount and asserting that analyze() returns no violations.
Reuse the existing expect import and keep the test focused on accessibility
coverage.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 213cf7ce-a6f6-46dd-af6e-2b203c12716c

📥 Commits

Reviewing files that changed from the base of the PR and between 0d1949f and 1af9369.

📒 Files selected for processing (14)
  • .changeset/fresh-buttons-rest.md
  • packages/core/src/components/chip/test/chip.ct.ts
  • packages/core/src/components/icon-button/test/icon-button.spec.tsx
  • packages/core/src/components/menu-category/test/menu-category.ct.ts
  • packages/core/src/components/menu/test/menu.ct.ts
  • packages/core/src/components/popover/test/popover.ct.ts
  • packages/core/src/components/tabs/tabs.tsx
  • packages/core/src/components/tabs/test/tabs.ct.ts
  • packages/core/src/components/toggle/test/toggle.ct.ts
  • packages/core/src/components/toggle/toggle.tsx
  • packages/core/src/components/utils/a11y.ts
  • packages/core/src/components/utils/internal/mixins/accessibility/aria-attribute-interceptors.spec.ts
  • packages/core/src/components/utils/internal/mixins/accessibility/aria-attribute-interceptors.ts
  • packages/core/src/components/utils/internal/mixins/accessibility/inherit-aria-attributes.mixin.ts

Comment thread .changeset/fresh-buttons-rest.md Resolved
nuke-ellington
nuke-ellington previously approved these changes Jul 30, 2026
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.changeset/mighty-poems-behave.md:
- Around line 5-8: Add the tracked requirement reference IX-111 to the pull
request description or a commit message associated with this change, preserving
the existing changeset content.

In `@packages/core/src/components/dropdown/dropdown.tsx`:
- Line 262: Update registerListener and the related trigger-resolution flow to
increment triggerResolutionToken before the falsy-trigger early return, clear
triggerElement when no trigger exists, and validate the request token before
applying checkForSubmenuAnchor or other submenu/z-index side effects. In
disconnectedCallback, invalidate pending resolutions and dispose any registered
listeners so stale async work cannot attach handlers after teardown.

In `@packages/core/src/components/menu-about/menu-about.tsx`:
- Around line 104-105: Update the first-item initialization and mapped tab props
in the menu-about component to reuse resolveTabKey() for legacy items whose
tabKey is undefined. Ensure the resolved key is passed as the synthetic
wrapper’s tabKey and used consistently for selected matching, while preserving
existing behavior for items that already define tabKey.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c32a81bc-7abe-4e31-8caa-bab00f83ffdf

📥 Commits

Reviewing files that changed from the base of the PR and between 1af9369 and 1de309e.

📒 Files selected for processing (7)
  • .changeset/mighty-poems-behave.md
  • packages/core/src/components/date-picker/date-picker.tsx
  • packages/core/src/components/dropdown/dropdown.tsx
  • packages/core/src/components/menu-about/menu-about.tsx
  • packages/core/src/components/tab-set/tab-set.tsx
  • packages/core/src/components/tabs/tab-key.ts
  • packages/core/src/components/tree/test/tree.ct.ts

Comment thread .changeset/mighty-poems-behave.md Resolved
Comment on lines +104 to +105
const firstItem = this.items[0];
this.activeTabKey = resolveTabKey(firstItem);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate relevant files =="
git ls-files | rg '(^|/)menu-about\.tsx$|(^|/)menu-about\.scss$|test/.*menu-about|TabSet|ix-tab|ix-tab-item' | sed -n '1,200p'

echo
echo "== menu-about outline =="
ast-grep outline packages/core/src/components/menu-about/menu-about.tsx --view expanded || true

echo
echo "== relevant menu-about lines =="
sed -n '1,240p' packages/core/src/components/menu-about/menu-about.tsx

echo
echo "== search tab-key resolver/usages =="
rg -n "resolveTabKey|tab-key|tabKey|activeTabKey|selected" packages/core/src/components packages/core -g '*.ts' -g '*.tsx' | sed -n '1,240p'

Repository: siemens/ix

Length of output: 31971


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate resolveTabKey =="
git ls-files | rg 'tab-key\.ts|tabs/.*tab-key'

echo
echo "== tab-key implementation =="
for f in $(git ls-files | rg 'tab-key\.ts$|tab-key/index\.ts|tab-key\.tsx$'); do
  echo "--- $f"
  sed -n '1,220p' "$f"
done

echo
echo "== menu-about-item implementation =="
sed -n '1,220p' packages/core/src/components/menu-about-item/menu-about-item.tsx

echo
echo "== related tests =="
sed -n '1,260p' packages/core/src/components/menu-about/test/menu-about.ct.ts

echo
echo "== search ix-tab-item/tabKey/selected =="
rg -n 'resolveTabKey|tab-key|tabKey|selected' packages/core/src/components/*/test packages/vue/src/components/ix-tab*.ts packages/vue/src/components/ix-tabs.ts packages/core/src/components -g '*.ts' -g '*.tsx' | sed -n '1,320p'

Repository: siemens/ix

Length of output: 44215


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate BaseTabMixin =="
git ls-files | rg 'tab\.mixin(\.ts)?$|BaseTabMixin'

echo
echo "== BaseTabMixin implementation =="
sed -n '1,160p' packages/core/src/components/tab-item/tab.mixin.ts

echo
echo "== tabKey prop declaration/usages =="
rg -n 'tabKey|tab-key' packages/core/src/utils packages/core/src/components/tab-item packages/core/src/components/tab-panel packages/core/src/components/tab-item packages/vue/src/components packages/core/src/components/menu-about packages/core/src/components/menu-about-item -g '*.ts' -g '*.tsx' | sed -n '1,220p'

echo
echo "== read-only JS/TS probe of resolveKey/render map behavior =="
node - <<'JS'
function resolveTabKey(element) {
  return element.tabKey ?? element.getAttribute('tab-key') ?? undefined;
}
for (const firstItem of [
  { tabKey: 'tab-1' },
  { tabKey: undefined, getAttribute(name) {
      return name === 'tab-key' ? 'tab-1' : null;
    }
  }
]) {
  const activeTabKey = resolveTabKey(firstItem);
  const generatedTab = {
    tabKey: firstItem.tabKey,
    selected: firstItem.tabKey === activeTabKey,
  };
  console.log(JSON.stringify({ activeTabKey, generatedTab }));
}
JS

Repository: siemens/ix

Length of output: 174


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate tabs mixin files =="
git ls-files | rg 'mixin|tab' | rg 'packages/core/src/components/(tabs/tab-item|tab-item|tabs)' | sed -n '1,120p'

echo
echo "== search BaseTabMixin definition =="
rg -n "class BaseTabMixin|interface BaseTabMixin|type BaseTabMixin|const BaseTabMixin|BaseTabMixinContract" packages/core/src packages -g '*.ts' -g '*.tsx' | sed -n '1,200p'

echo
echo "== read-only JS probe of resolveKey/render map behavior =="
node - <<'JS'
function resolveTabKey(element) {
  return element.tabKey ?? element.getAttribute('tab-key') ?? undefined;
}
const cases = [
  {
    name: 'tabKey property present',
    element: { tabKey: 'tab-1', getAttribute: () => null },
  },
  {
    name: 'tab-key attribute only, property undefined',
    element: { tabKey: undefined, getAttribute(name) {
      return name === 'tab-key' ? 'tab-1' : null;
    }},
  },
];
for (const { name, element } of cases) {
  const activeTabKey = resolveTabKey(element);
  const generatedTab = {
    tabKey: element.tabKey,
    selected: element.tabKey === activeTabKey,
  };
  console.log(name, JSON.stringify({ activeTabKey, generatedTab }));
}
JS

Repository: siemens/ix

Length of output: 3045


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tab mixin implementation =="
sed -n '1,160p' packages/core/src/components/tab-item/tab.mixin.tsx

Repository: siemens/ix

Length of output: 899


Make legacy item tabKey property match the resolved active key.

resolveTabKey() falls back to the tab-key attribute when tabKey is undefined, but ix-tab-item still receives only firstItem.tabKey. With an attribute-only legacy item, activeTabKey becomes the correct value while the generated tab is not selected. Use resolveTabKey() for both activeTabKey assignment and the mapped tabKey/selected props so the synthetic wrapper has the same key that TabSet can match.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/components/menu-about/menu-about.tsx` around lines 104 -
105, Update the first-item initialization and mapped tab props in the menu-about
component to reuse resolveTabKey() for legacy items whose tabKey is undefined.
Ensure the resolved key is passed as the synthetic wrapper’s tabKey and used
consistently for selected matching, while preserving existing behavior for items
that already define tabKey.

nuke-ellington
nuke-ellington previously approved these changes Jul 31, 2026
@lzeiml
lzeiml requested a review from a team as a code owner October 2, 2026 09:51
@lzeiml
lzeiml requested a review from alexkaduk October 2, 2026 09:51
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants