Skip to content
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 .changeset/fresh-buttons-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@siemens/ix': patch
---

Forward updated and removed ARIA attributes to each component's accessible element without leaving invalid host copies. `ix-toggle` now keeps its component managed attributes (`role`, `aria-checked`, `aria-disabled`, `aria-required`) intact while still accepting custom ARIA attributes such as `aria-label`.
Comment thread
nuke-ellington marked this conversation as resolved.
8 changes: 8 additions & 0 deletions .changeset/mighty-poems-behave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@siemens/ix': patch
---

Fixed `ix-dropdown` trigger handling: an empty `trigger` value no longer matches
elements without an `id`, changing the trigger to an empty value now removes the
previously registered listeners, and asynchronously resolved triggers that became
stale are discarded instead of attaching orphaned listeners.
Comment on lines +5 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

๐Ÿ“ Maintainability & Code Quality | ๐ŸŸก Minor | โšก Quick win

Add the required requirement reference.

The PR context lists EIX-111, but the Siemens IX rule requires a GitHub issue or Jira reference in IX-<number> form in the PR description or a commit message. Add the required reference before merge.

As per path instructions: โ€œRequire GitHub issue or Jira reference (IX-) in PR description or commit message when work is tied to a tracked requirement.โ€

๐Ÿงฐ Tools
๐Ÿช› markdownlint-cli2 (0.23.1)

[warning] 5-5: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

๐Ÿค– 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 @.changeset/mighty-poems-behave.md around lines 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.

Source: Path instructions

5 changes: 5 additions & 0 deletions .changeset/persistent-pinned-menu.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@siemens/ix': patch
---

Keep pinned `ix-menu` components expanded when a menu item is selected.
5 changes: 5 additions & 0 deletions .changeset/reliable-tab-activation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@siemens/ix': patch
---

Fix `ix-tab-set` panel activation and `ix-menu-about` tab activation when child components are still initializing.
5 changes: 5 additions & 0 deletions .changeset/stable-date-picker-focus.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@siemens/ix': patch
---

Fix `ix-date-picker` keyboard navigation so focus consistently moves to the expected day.
5 changes: 5 additions & 0 deletions .changeset/steady-dropdown-triggers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@siemens/ix': patch
---

Fix `ix-dropdown` trigger initialization so dropdowns open reliably immediately after rendering or when triggers are added dynamically.
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,6 @@ regressionTest(
await expect(
chatInput.locator('ix-dropdown-button.attachment-overflow')
).toHaveCount(0);
await expect(chatInput.locator('.attachments')).not.toHaveClass(
/has-attachment-scrollbar/
);
await expect(
page.locator('[data-attachment-overflow-generated]')
).toHaveCount(0);
Expand Down
50 changes: 24 additions & 26 deletions packages/core/src/components/checkbox/tests/checkbox.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,7 @@ regressionTest(
const checkbox = page.locator('ix-checkbox');
await expect(checkbox).not.toHaveClass(/label-less/);
await expect(checkbox).toHaveText(/Custom slot label text/);
const width = await checkbox.evaluate((element) =>
Number.parseFloat(getComputedStyle(element).width)
);
expect(width).toBeGreaterThan(24);
await expect(checkbox.locator('ix-typography')).toBeVisible();
}
);

Expand All @@ -121,37 +118,38 @@ regressionTest('label', async ({ mount, page }) => {
await expect(checkboxElement).toHaveText(/some label/);
});

test('Checkbox should not cause layout shift when checked', async ({
mount,
page,
}) => {
await mount(`
regressionTest(
'Checkbox should not cause layout shift when checked',
async ({ mount, page }) => {
await mount(`
<ix-checkbox label="test"></ix-checkbox>
<div id="element-below">This element should not move</div>
`);

await page.waitForSelector('ix-checkbox', { state: 'attached' });
const checkbox = page.locator('ix-checkbox');
const elementBelow = page.locator('#element-below');

const initialBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});
await expect(checkbox).toHaveClass(/hydrated/);
await expect(elementBelow).toBeVisible();
await page.evaluate(() => document.fonts.ready);

await page.click('ix-checkbox');
const initialBounds = await elementBelow.boundingBox();
if (!initialBounds) {
throw new Error('Expected element below checkbox to have a bounding box');
}

await page.waitForFunction(() => {
const checkbox = document.querySelector('ix-checkbox');
return checkbox?.getAttribute('aria-checked') === 'true';
});
await checkbox.click();
await expect(checkbox).toHaveAttribute('aria-checked', 'true');

const newBounds = await page.$eval('#element-below', (el) => {
const rect = el.getBoundingClientRect();
return { top: rect.top, left: rect.left };
});
const newBounds = await elementBelow.boundingBox();
if (!newBounds) {
throw new Error('Expected element below checkbox to remain visible');
}

expect(newBounds.top).toBeCloseTo(initialBounds.top, 0);
expect(newBounds.left).toBeCloseTo(initialBounds.left, 0);
});
expect(newBounds.y).toBeCloseTo(initialBounds.y, 0);
expect(newBounds.x).toBeCloseTo(initialBounds.x, 0);
}
);

test.describe('accessibility', () => {
test('should expose aria-label for accessibility queries', async ({
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/components/chip/test/chip.ct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ regressionTest(
await expect(
page.getByRole('button', { name: 'Close chip' })
).toBeVisible();

await chip.evaluate((element) => {
element.setAttribute('aria-label', 'Project Beta');
});

await expect(chip).not.toHaveAttribute('aria-label');
await expect(
page.getByRole('button', { name: 'Project Beta' })
).toBeVisible();
await expect(chip).toHaveAttribute('role', 'group');
}
);

Expand Down
18 changes: 15 additions & 3 deletions packages/core/src/components/date-picker/date-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ export class DatePicker
break;
}

event.preventDefault();
return;
}

Expand Down Expand Up @@ -847,7 +848,18 @@ export class DatePicker
return rows;
}

public changeFocusedDay() {
public changeFocusedDay(focusTarget?: EventTarget) {
const focusedDayElement =
focusTarget instanceof HTMLElement
? focusTarget.closest<HTMLElement>('[data-calendar-day]')
: null;
const focusedDay = focusedDayElement?.dataset.calendarDay;

if (focusedDay) {
this.focusedDay = Number.parseInt(focusedDay, 10);
return;
}

if (this.monthChangedFromFocus) {
return;
}
Expand Down Expand Up @@ -922,9 +934,9 @@ export class DatePicker
return (
<Host
onKeyDown={(event: KeyboardEvent) => this.onKeyDown(event)}
onFocusin={() => {
onFocusin={(event: FocusEvent) => {
if (hasKeyboardMode()) {
this.changeFocusedDay();
this.changeFocusedDay(event.composedPath()[0]);
}
}}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1450,8 +1450,14 @@ regressionTest('form-ready - initial value', async ({ mount, page }) => {
const formElement = page.locator('form');
preventFormSubmission(formElement);

const formData = await getFormValue(formElement, 'appointment-time', page);
expect(formData).toBe('2024/12/25 10:00:00');
const dateTimeInput = page.locator('ix-datetime-input');
await expect(dateTimeInput).toHaveClass(/hydrated/);
await expect(dateTimeInput.getByRole('textbox')).toHaveValue(
'2024/12/25 10:00:00'
);
await expect
.poll(() => getFormValue(formElement, 'appointment-time', page))
.toBe('2024/12/25 10:00:00');
});

regressionTest(
Expand Down
29 changes: 27 additions & 2 deletions packages/core/src/components/dropdown/dropdown-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface DropdownInterface extends IxComponentInterface {

getAssignedSubmenuIds(): string[];
getId(): string;
matchesTrigger(eventTargets: EventTarget[]): boolean;

discoverSubmenu(): void;

Expand Down Expand Up @@ -139,12 +140,36 @@ class DropdownController {
);
}

private getDropdownByTriggerPath(eventTargets: EventTarget[]) {
for (const dropdown of this.stack.values()) {
if (dropdown.matchesTrigger(eventTargets)) {
return dropdown;
}
}

return undefined;
}

private addOverlayListeners() {
this.isWindowListenerActive = true;

window.addEventListener('click', (event: MouseEvent) => {
const hasTrigger = this.pathIncludesTrigger(event.composedPath());
const hasDropdown = this.pathIncludesDropdown(event.composedPath());
const eventTargets = event.composedPath();
const hasTrigger = this.pathIncludesTrigger(eventTargets);
const hasDropdown = this.pathIncludesDropdown(eventTargets);

if (!hasTrigger && !event.defaultPrevented) {
const dropdown = this.getDropdownByTriggerPath(eventTargets);
if (dropdown) {
if (dropdown.isPresent()) {
this.dismiss(dropdown);
} else {
this.present(dropdown);
}
this.dismissOthers(dropdown.getId());
return;
}
}

if (!hasTrigger && !hasDropdown) {
this.dismissAll();
Expand Down
83 changes: 74 additions & 9 deletions packages/core/src/components/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ export class Dropdown
private readonly dialogRef = makeRef<HTMLDialogElement>();
private intersectObserverTrigger?: IntersectionObserver;
private triggerElement?: Element;
private triggerResolutionToken = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

๐Ÿฉบ Stability & Availability | ๐ŸŸ  Major

Invalidate all stale trigger resolutions.

registerListener returns before incrementing triggerResolutionToken. If an asynchronous resolution is pending and trigger changes to '' or undefined, the old request still has the current token. Lines 547-552 can then attach listeners for the old trigger after changedTrigger disposed them.

The early return also leaves triggerElement pointing to the old element. In addition, resolveElement() calls checkForSubmenuAnchor() before the token check, so stale requests can still set isSubMenu and the dropdown z-index.

Increment the token before the falsy guard, clear triggerElement, and apply submenu side effects only after validating the current request. Also invalidate pending requests and dispose listeners in disconnectedCallback.

Proposed token fix
   private async registerListener(element: ElementReference) {
+    const resolutionToken = ++this.triggerResolutionToken;
+
     if (!element) {
+      this.triggerElement = undefined;
       return;
     }
 
-    const resolutionToken = ++this.triggerResolutionToken;

Also applies to: 523-552, 754-765

๐Ÿค– 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` at 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.

private anchorElement?: Element;
private forwardQueryElement: HTMLElement | null = null;
private dropdownElementId = `dropdown-${sequenceId++}`;
Expand Down Expand Up @@ -316,6 +317,20 @@ export class Dropdown
return this.dropdownElementId;
}

matchesTrigger(eventTargets: EventTarget[]) {
const trigger =
this.trigger ?? this.hostElement.getAttribute('trigger') ?? undefined;

return eventTargets.some(
(target) =>
target === trigger ||
(typeof trigger === 'string' &&
trigger !== '' &&
target instanceof HTMLElement &&
target.id === trigger)
);
}
Comment thread
danielleroux marked this conversation as resolved.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
willDismiss() {
const { defaultPrevented } = this.showChange.emit(false);
return !defaultPrevented;
Expand Down Expand Up @@ -505,12 +520,36 @@ export class Dropdown
}

private async registerListener(element: ElementReference) {
this.triggerElement = await this.resolveElement(element);
if (!element) {
return;
}

if (!this.triggerElement) {
const resolutionToken = ++this.triggerResolutionToken;
const immediateElement = this.resolveImmediateElement(element);
const canRegisterImmediately =
immediateElement &&
(!hasDropdownItemWrapperImplemented(immediateElement) ||
immediateElement.tagName === 'IX-DROPDOWN-ITEM');

if (canRegisterImmediately) {
this.triggerElement = immediateElement;
if (immediateElement.tagName === 'IX-DROPDOWN-ITEM') {
(immediateElement as HTMLIxDropdownItemElement).isSubMenu = true;
this.hostElement.style.zIndex = `var(--theme-z-index-dropdown)`;
}
this.addEventListenersFor();
this.discoverSubmenu();
return;
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
const resolvedElement = await this.resolveElement(element);

if (!resolvedElement || resolutionToken !== this.triggerResolutionToken) {
return;
}

this.triggerElement = resolvedElement;

this.addEventListenersFor();
this.discoverSubmenu();
}
Expand Down Expand Up @@ -586,6 +625,32 @@ export class Dropdown
return this.checkForSubmenuAnchor(el);
}

private resolveImmediateElement(
element: ElementReference
): HTMLElement | undefined {
if (element instanceof Promise) {
return undefined;
}

if (element instanceof HTMLElement) {
return element;
}

const documentElement = document.getElementById(element);
if (documentElement) {
return documentElement;
}

const root = this.hostElement.getRootNode();
if (root instanceof ShadowRoot) {
return (
root.querySelector<HTMLElement>(`#${CSS.escape(element)}`) ?? undefined
);
}

return undefined;
}

private async checkForSubmenuAnchor(element?: Element) {
if (!element) {
return undefined;
Expand Down Expand Up @@ -686,18 +751,18 @@ export class Dropdown
}

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

this.registerListener(newTriggerValue);
await this.registerListener(newTriggerValue);
}

private applyFallbackPosition(element: HTMLElement) {
Expand All @@ -706,9 +771,9 @@ export class Dropdown
this.hostElement.parentElement || this.hostElement;
const refRect = referenceElement.getBoundingClientRect();

const transform = `translate(${Math.round(
refRect.left
)}px, ${Math.round(refRect.top)}px)`;
const transform = `translate(${Math.round(refRect.left)}px, ${Math.round(
refRect.top
)}px)`;

Object.assign(element.style, {
top: '0',
Expand Down Expand Up @@ -864,7 +929,7 @@ export class Dropdown
return;
}

this.changedTrigger(this.trigger, undefined);
await this.changedTrigger(this.trigger, undefined);
}

override async componentDidRender() {
Expand Down
Loading
Loading