fix(core/dropdown-button): accessibility consistency in dropdown-button - #2585
fix(core/dropdown-button): accessibility consistency in dropdown-button#2585GayatriK2002 wants to merge 6 commits into
Conversation
✅ Deploy Preview for ix-storybook canceled.
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Vue preview examples for the dropdown button components (dropdown-button-icon.vue and dropdown-button.vue) to import IxDropdownItem alongside IxDropdownButton from @siemens/ix-vue. There are no review comments to address, and I have no additional feedback to provide.
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.
📝 WalkthroughWalkthroughThis PR updates Vue prop handling and Stencil runtime patches, adds dropdown preview alignment and Vue imports, and changes selected TypeScript configurations from ES2017 to ES2020. ChangesVue Sentinel and Patch Workflow
Dropdown Preview Fixes
ES2020 TypeScript Configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
patches/@stencil__vue-output-target.patch (1)
1-29:⚠️ Potential issue | 🟡 MinorAdd documentation for the
@stencil/vue-output-targetpatch topatches/README.md.Following the existing pattern for the
@stencil/corepatch, document this patch by adding:
- The upstream issue being worked around (related to Vue <3.1.0 compatibility with Symbol())
- The version of
@stencil/vue-output-targetthis targets (^0.13.2)- When/if an upstream fix is available to remove this patch
This will help future maintainers understand when the patch can be safely removed.
🤖 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 `@patches/`@stencil__vue-output-target.patch around lines 1 - 29, Add documentation for the `@stencil/vue-output-target` patch to the patches/README.md file following the existing pattern used for the `@stencil/core` patch entry. Include the upstream issue being addressed (Vue <3.1.0 compatibility issue with Symbol()), specify the version of `@stencil/vue-output-target` being patched (^0.13.2), and add a note about when this patch can be safely removed once an upstream fix is available. This will ensure future maintainers understand the purpose of the EMPTY_PROP modification from Symbol() to {} and can track when the patch is no longer needed.
🤖 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/angular/tsconfig.json`:
- Line 19: The `lib` configuration on line 19 is set to ["dom", "es2020"] while
the `target` configuration on line 32 is set to "es2015", creating a mismatch
where ES2020 APIs are permitted during type-checking but the code is only
transpiled to ES2015. Either update both `lib` and `target` to consistently use
"es2020" since the Angular 20.0.0+ peer dependency requires ES2020+ runtime
support, or if keeping the current transpilation target, update the `lib` to
["dom", "es2015"] and add explicit documentation in a README or contributing
guide clearly stating that the library requires an ES2020+ runtime environment
due to its Angular 20.0.0+ peer dependency.
---
Outside diff comments:
In `@patches/`@stencil__vue-output-target.patch:
- Around line 1-29: Add documentation for the `@stencil/vue-output-target` patch
to the patches/README.md file following the existing pattern used for the
`@stencil/core` patch entry. Include the upstream issue being addressed (Vue
<3.1.0 compatibility issue with Symbol()), specify the version of
`@stencil/vue-output-target` being patched (^0.13.2), and add a note about when
this patch can be safely removed once an upstream fix is available. This will
ensure future maintainers understand the purpose of the EMPTY_PROP modification
from Symbol() to {} and can track when the patch is no longer needed.
🪄 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: f4064c63-0c15-44d6-b1c9-fa8f3396f1a2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
package.jsonpackages/angular-standalone-test-app/src/preview-examples/dropdown-button-icon.csspackages/angular-test-app/src/preview-examples/dropdown-button-icon.csspackages/angular/tsconfig.jsonpackages/html-test-app/src/preview-examples/dropdown-button-icon.csspackages/nextjs-test-app/tsconfig.jsonpackages/react-test-app/src/preview-examples/dropdown-button-icon.scoped.csspackages/vue-test-app/src/preview-examples/dropdown-button-icon.csspackages/vue-test-app/src/preview-examples/dropdown-button-icon.vuepackages/vue-test-app/src/preview-examples/dropdown-button.vuepackages/vue/src/vue-component-lib/utils.tspackages/vue/tsconfig.jsonpatches/@stencil__vue-output-target.patchtesting/visual-testing/tsconfig.test.json
| "experimentalDecorators": true, | ||
| "forceConsistentCasingInFileNames": true, | ||
| "lib": ["dom", "es2017"], | ||
| "lib": ["dom", "es2020"], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TypeScript, what are the runtime implications of setting "compilerOptions.lib": ["dom","es2020"] while keeping "compilerOptions.target": "es2015", and what polyfill strategy is recommended for published libraries?
💡 Result:
In TypeScript, the "lib" and "target" compiler options serve distinct, non-overlapping roles [1][2]. The "target" option determines the ECMAScript version to which your TypeScript code is transpiled (e.g., downleveling arrow functions to standard functions) [3][4]. Conversely, "lib" specifies which type definitions are included in the compilation, which dictates which global JavaScript APIs (like "document" or "Promise") are available for the compiler to recognize during type-checking [1][4]. Setting "compilerOptions.lib" to ["dom", "es2020"] while keeping "compilerOptions.target" at "es2015" creates a mismatch between what the compiler recognizes and what it emits [2][5]. Specifically: 1. Runtime Implications: The compiler will permit the use of ES2020 APIs (e.g., BigInt, String.prototype.matchAll) and DOM APIs because they are defined in "lib" [1][2][6]. However, because "target" is set to "es2015", TypeScript will not automatically downlevel ES2020 syntax (if any were used) or provide polyfills for these features [3][5]. Your code will run in an ES2015-compatible environment, but it will throw runtime errors if it attempts to call an ES2020 API that does not exist natively in the target environment without accompanying polyfills [2][6]. 2. Polyfill Strategy for Published Libraries: It is highly recommended that library authors avoid bundling polyfills directly in their packages [7]. Bundling polyfills can lead to namespace pollution (if global polyfills are used) and significantly bloat bundle sizes for consumers who may already provide their own polyfills [8][7]. Recommended practices include: - Document Requirements: Clearly document the minimum runtime environment requirements (e.g., browsers supporting ES2020, or Node.js versions) in the library's README [8][7]. - Use "Pure" Polyfills: If necessary, use tools like @babel/plugin-transform-runtime, which injects "pure" polyfills (imported as modules) that do not pollute the global scope [9][7]. - Rely on Peer Dependencies: Alternatively, instruct users to provide necessary polyfills themselves, a standard pattern for many large libraries [7]. - Keep "lib" and "target" Synced: Unless you have specific reasons to decouple them (e.g., advanced polyfill management), it is best practice to keep "lib" and "target" roughly in sync to ensure your code's type-checked features match your runtime capabilities [5].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/lib.html
- 2: https://www.claritician.com/typescript-lib-vs-target-what-s-the-difference
- 3: https://www.typescriptlang.org/tsconfig/target.html
- 4: https://microsoft.github.io/TypeScript-New-Handbook/reference/compiler-options/
- 5: https://www.totaltypescript.com/workshops/typescript-pro-essentials/configuring-typescript/understanding-lib-and-target-in-typescript-configuration
- 6: https://runebook.dev/en/docs/typescript/tsconfig/lib-config
- 7: https://medium.com/@lee_85949/polyfilling-a-javascript-library-the-right-way-337806a54152
- 8: https://typescript.page/when-to-replace-chrome-like-defaults-in-your-stack-choosing-
- 9: https://github.com/babel/babel-polyfills/
🏁 Script executed:
# First, let's find and read the tsconfig.json file in question
fd -t f "tsconfig.json" packages/angular
# Read the specific file with context around lines 19 and 32
if [ -f "packages/angular/tsconfig.json" ]; then
echo "=== Full tsconfig.json content ==="
cat -n packages/angular/tsconfig.json
fiRepository: siemens/ix
Length of output: 1582
🏁 Script executed:
# Check for ng-packagr configuration
fd -t f "ng-packagr.json|package.json" packages/angular | head -5
# Read the package.json to understand the library setup
if [ -f "packages/angular/package.json" ]; then
echo "=== packages/angular/package.json ==="
cat packages/angular/package.json
fi
# Check for any README or documentation about requirements
fd -t f "README.md" packages/angularRepository: siemens/ix
Length of output: 1676
🏁 Script executed:
# Read the README
if [ -f "packages/angular/README.md" ]; then
echo "=== packages/angular/README.md ==="
cat packages/angular/README.md
fi
# Check the main ng-package.json
if [ -f "packages/angular/ng-package.json" ]; then
echo -e "\n=== packages/angular/ng-package.json ==="
cat packages/angular/ng-package.json
fiRepository: siemens/ix
Length of output: 621
Align lib and target configuration or explicitly document runtime ES2020+ requirement.
The lib: ["dom", "es2020"] on line 19 with target: "es2015" on line 32 creates a mismatch where the compiler permits ES2020 APIs during type-checking but only transpiles to ES2015 syntax. While the library's peer dependency on @angular/core >=20.0.0 implies ES2020+ runtime support, this alignment gap should be resolved to prevent accidental API usage that could fail in edge cases and to make runtime requirements explicit. Either sync lib and target to "es2020" or add explicit documentation of the minimum runtime environment (ES2020+ support required due to Angular 20.0.0+ peer dependency).
🤖 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/angular/tsconfig.json` at line 19, The `lib` configuration on line
19 is set to ["dom", "es2020"] while the `target` configuration on line 32 is
set to "es2015", creating a mismatch where ES2020 APIs are permitted during
type-checking but the code is only transpiled to ES2015. Either update both
`lib` and `target` to consistently use "es2020" since the Angular 20.0.0+ peer
dependency requires ES2020+ runtime support, or if keeping the current
transpilation target, update the `lib` to ["dom", "es2015"] and add explicit
documentation in a README or contributing guide clearly stating that the library
requires an ES2020+ runtime environment due to its Angular 20.0.0+ peer
dependency.
Source: Coding guidelines
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (2)
54-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the Yarn-specific
postinstall-postinstalldependency, or verify a Yarn 1 requirement.
postinstall-postinstallis documented bypatch-packagefor Yarn 1 behavior, while this repository’s scripts use pnpm; pnpm also has native dependency patching support. The added package appears unnecessary for the shown workflow. (npmjs.com)🤖 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 `@package.json` around lines 54 - 55, Remove the Yarn-specific postinstall-postinstall dependency from the dependencies near patch-package, unless the repository explicitly requires Yarn 1; if Yarn 1 is required, verify and document that requirement while preserving the existing pnpm workflow.
33-34: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep
patch-packageon one lifecycle hook
pnpm installruns bothprepareandpostinstallat the workspace root, so this applies the patch step twice. Keep a single hook here—postinstallis enough—and drop thepostinstall-postinstallhelper unless you need Yarn-specific behavior.🤖 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 `@package.json` around lines 33 - 34, Remove the patch-package invocation from the prepare script in package.json, keeping it only under postinstall so pnpm install applies patches once. Also remove the postinstall-postinstall helper unless it is required for Yarn-specific behavior.
🤖 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.
Outside diff comments:
In `@package.json`:
- Around line 54-55: Remove the Yarn-specific postinstall-postinstall dependency
from the dependencies near patch-package, unless the repository explicitly
requires Yarn 1; if Yarn 1 is required, verify and document that requirement
while preserving the existing pnpm workflow.
- Around line 33-34: Remove the patch-package invocation from the prepare script
in package.json, keeping it only under postinstall so pnpm install applies
patches once. Also remove the postinstall-postinstall helper unless it is
required for Yarn-specific behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f1a43dd-92e7-4f81-b053-9995ad01dd43
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonpackages/vue/tsconfig.json
|
Closing this PR due to persistent merge conflicts that were difficult to resolve. I've opened a new PR, #2654 , with the same changes. Please continue the review there. |
1 participant
💡 What is the current behavior?
GitHub Issue Number: #
Jira Issue Number: 4336
🆕 What is the new behavior?
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support
Summary by CodeRabbit
Bug Fixes
Chores