Skip to content

fix(core/dropdown-button): accessibility consistency in dropdown-button-icon #2654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GayatriK2002 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(core/dropdown-button): accessibility consistency in dropdown-button-icon #2654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bef1f8
added patch for vue wrapper
GayatriK2002 Jul 15, 2026
3735a7c
updated package.json according to review comments
GayatriK2002 Jul 15, 2026
40a0a1a
Merge branch 'main' into fix-4336/dropdown-accessibility
GayatriK2002 Jul 15, 2026
1e06d47
Merge branch 'main' into fix-4336/dropdown-accessibility
GayatriK2002 Jul 16, 2026
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
1 change: 1 addition & 0 deletions packages/angular-standalone-test-app/src/preview-examples/dropdown-button.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.dropdown-button {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
position: relative;
max-width: 20rem;
Expand Down
1 change: 1 addition & 0 deletions packages/angular-test-app/src/preview-examples/dropdown-button.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.dropdown-button {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
position: relative;
max-width: 20rem;
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"declarationDir": "dist",
"experimentalDecorators": true,
"forceConsistentCasingInFileNames": true,
"lib": ["dom", "es2017"],
"lib": ["dom", "es2020"],
"module": "es2015",
"moduleResolution": "bundler",
"noImplicitAny": true,
Expand Down
1 change: 1 addition & 0 deletions packages/html-test-app/src/preview-examples/dropdown-button.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.dropdown-button {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
position: relative;
max-width: 20rem;
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs-test-app/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compilerOptions": {
"target": "ES2017",
"target": "ES2020",
"lib": [
"dom",
"dom.iterable",
Expand Down
1 change: 1 addition & 0 deletions packages/react-test-app/src/preview-examples/dropdown-button.scoped.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.dropdown-button {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
position: relative;
max-width: 20rem;
Expand Down
2 changes: 1 addition & 1 deletion packages/vue-test-app/src/preview-examples/dropdown-button-icon.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<script setup lang="ts">
import { iconCheckboxes } from '@siemens/ix-icons/icons';
import { IxDropdownButton } from '@siemens/ix-vue';
import { IxDropdownButton, IxDropdownItem} from '@siemens/ix-vue';
</script>

<style scoped src="./dropdown-button-icon.css"></style>
Expand Down
1 change: 1 addition & 0 deletions packages/vue-test-app/src/preview-examples/dropdown-button.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
.dropdown-button {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
position: relative;
max-width: 20rem;
Expand Down
2 changes: 1 addition & 1 deletion packages/vue-test-app/src/preview-examples/dropdown-button.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<script setup lang="ts">
import { iconCheckboxes } from '@siemens/ix-icons/icons';
import { IxDropdownButton } from '@siemens/ix-vue';
import { IxDropdownButton, IxDropdownItem } from '@siemens/ix-vue';
</script>

<style scoped src="./dropdown-button.css"></style>
Expand Down
4 changes: 2 additions & 2 deletions packages/vue/src/vue-component-lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
* and then check if it is not undefined for Vue >= 3.1.0.
* See https://github.com/vuejs/vue-next/issues/3889
*/
const EMPTY_PROP = Symbol();
const EMPTY_PROP = {};

@gemini-code-assist gemini-code-assist Bot Jul 15, 2026

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.

medium

Since EMPTY_PROP is now defined as a plain object {} instead of a unique Symbol(), it is mutable. To prevent accidental mutation of this sentinel value elsewhere in the codebase, it is highly recommended to freeze the object using Object.freeze().

Suggested change
const EMPTY_PROP = {};
const EMPTY_PROP = Object.freeze({});

@GayatriK2002 GayatriK2002 Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[EMPTY_PROP] is only used as a sentinel compared by reference equality [value !== EMPTY_PROP] and is never mutated anywhere, so freezing isn't functionally required. I've left it as a plain {} for now to keep the patch minimal and aligned with the upstream source.

const DEFAULT_EMPTY_PROP = { default: EMPTY_PROP };

interface NavManager<T = any> {

Check warning on line 36 in packages/vue/src/vue-component-lib/utils.ts

View workflow job for this annotation

GitHub Actions / build 

Unexpected any. Specify a different type
navigate: (options: T) => void;
}

Expand Down Expand Up @@ -65,7 +65,7 @@
*/
export const defineContainer = <Props, VModelType = string | number | boolean>(
name: string,
defineCustomElement: any,

Check warning on line 68 in packages/vue/src/vue-component-lib/utils.ts

View workflow job for this annotation

GitHub Actions / build 

Unexpected any. Specify a different type
componentProps: string[] = [],
modelProp?: string,
modelUpdateEvent?: string
Expand Down Expand Up @@ -112,7 +112,7 @@
* when ionChange bubbles up from Component B.
*/
if (e.target.tagName === el.tagName) {
modelPropValue = (e?.target as any)[modelProp];

Check warning on line 115 in packages/vue/src/vue-component-lib/utils.ts

View workflow job for this annotation

GitHub Actions / build 

Unexpected any. Specify a different type
emit(UPDATE_VALUE_EVENT, modelPropValue);
}
});
Expand All @@ -130,7 +130,7 @@
if (routerLink === EMPTY_PROP) return;

if (navManager !== undefined) {
const navigationPayload: any = { event: ev };

Check warning on line 133 in packages/vue/src/vue-component-lib/utils.ts

View workflow job for this annotation

GitHub Actions / build 

Unexpected any. Specify a different type
for (const key in props) {
const value = props[key];
if (
Expand Down Expand Up @@ -168,7 +168,7 @@
}
};

let propsToAdd: any = {

Check warning on line 171 in packages/vue/src/vue-component-lib/utils.ts

View workflow job for this annotation

GitHub Actions / build 

Unexpected any. Specify a different type
ref: containerRef,
class: getElementClasses(containerRef, classes),
onClick: handleClick,
Expand All @@ -185,7 +185,7 @@
if (
// eslint-disable-next-line no-prototype-builtins
(props.hasOwnProperty(key) && value !== EMPTY_PROP) ||
key.startsWith(ARIA_PROP_PREFIX)
(key.startsWith(ARIA_PROP_PREFIX) && value !== EMPTY_PROP)
) {
propsToAdd[key] = value;
}
Comment on lines 185 to 191

@coderabbitai coderabbitai Bot Jul 17, 2026

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 | 🟡 Minor | ⚡ Quick win

Add regression coverage for sentinel-valued and explicit ARIA props.

Test that omitted ARIA props never reach the custom element as a sentinel string, while explicit values such as aria-label="Menu" are still forwarded unchanged. Exercise the actual Vue runtime path as well as this utility if both implementations are shipped.

🤖 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/vue/src/vue-component-lib/utils.ts` around lines 185 - 191, Add
regression tests around the prop-forwarding logic containing ARIA_PROP_PREFIX
and EMPTY_PROP, covering omitted ARIA props being excluded rather than passed as
the sentinel string and explicit values such as aria-label="Menu" being
forwarded unchanged. Exercise the actual Vue runtime path and the utility
implementation when both are shipped.

Expand Down
2 changes: 1 addition & 1 deletion packages/vue/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"lib": ["dom", "es2020"],
"module": "es2015",
"moduleResolution": "bundler",
"target": "es2017",
"target": "es2020",
"skipLibCheck": true
}
}
26 changes: 26 additions & 0 deletions patches/@stencil__vue-output-target.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
diff --git a/dist/runtime.cjs.js b/dist/runtime.cjs.js
index 8d1b57d..8aaa91b 100644
--- a/dist/runtime.cjs.js
+++ b/dist/runtime.cjs.js
@@ -116,7 +116,7 @@ const ARIA_PROP_PREFIX = 'aria';
* and then check if it is not undefined for Vue >= 3.1.0.
* See https://github.com/vuejs/vue-next/issues/3889
*/
-const EMPTY_PROP = Symbol();
+const EMPTY_PROP = {};
const DEFAULT_EMPTY_PROP = { default: EMPTY_PROP };
const getComponentClasses = (classes) => {
return classes?.split(' ') || [];
diff --git a/dist/runtime.js b/dist/runtime.js
index 7efe4d9..4415241 100644
--- a/dist/runtime.js
+++ b/dist/runtime.js
@@ -114,7 +114,7 @@ const ARIA_PROP_PREFIX = 'aria';
* and then check if it is not undefined for Vue >= 3.1.0.
* See https://github.com/vuejs/vue-next/issues/3889
*/
-const EMPTY_PROP = Symbol();
+const EMPTY_PROP = {};
const DEFAULT_EMPTY_PROP = { default: EMPTY_PROP };
const getComponentClasses = (classes) => {
return classes?.split(' ') || [];
25 changes: 14 additions & 11 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pnpm-workspace.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ overrides:

patchedDependencies:
'@stencil/core': "patches/@stencil__core.patch"
'@stencil/vue-output-target': "patches/@stencil__vue-output-target.patch"

@coderabbitai coderabbitai Bot Jul 17, 2026

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 | 🟠 Major | ⚡ Quick win

Add a changeset for this accessibility behavior change.

This PR changes Vue wrapper behavior and user-facing accessibility output. Add a changeset for the affected package, or explicitly justify in the PR why the change is internal-only.

🤖 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 `@pnpm-workspace.yaml` at line 28, Add a changeset describing the Vue wrapper
accessibility behavior change and identifying the affected package with the
appropriate release bump; do not treat the patch entry for
`@stencil/vue-output-target` as sufficient release documentation.

Source: Path instructions

4 changes: 2 additions & 2 deletions testing/visual-testing/tsconfig.test.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
"allowUnreachableCode": false,
"declaration": true,
"experimentalDecorators": true,
"lib": ["dom", "es2017"],
"lib": ["dom", "es2020"],
"moduleResolution": "bundler",
"module": "esnext",
"target": "es2017",
"target": "es2020",
"noUnusedLocals": true,
"noUnusedParameters": true,
"preserveSymlinks": false,
Expand Down
Loading