Skip to content

Conversation

@anthony0t
Copy link
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

When parameter got changed for multi-select combo box, the selected keys are not updated in UI. The combo box selected key is updated when option changes for single selection, however we doesn't have the code to handle multi-selection combo box. This PR added the missing logic to handle multi-select scenario.

Impact of Change

  • Users:
    Now user see the new value shown correctly in UI.
  • Developers:
  • System:

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: local

Contributors

Screenshots/Videos

@anthony0t anthony0t added the risk:low Low risk change with minimal impact label Jan 16, 2026
Copilot AI review requested due to automatic review settings January 16, 2026 06:12
@github-actions
Copy link

github-actions bot commented Jan 16, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(ui): multi-select combobox selection not updated when options/keys change
  • Issue: None — title is concise, uses the conventional prefix, and clearly describes the problem and area affected.
  • Recommendation: None — good title.

Commit Type

  • Properly selected (fix).
  • Only one commit type selected which is correct.

Risk Level

  • The PR body marked Low and the PR has the risk:low label — these match. Based on the diff (small UI logic change + one unit test), the advised risk level is Low.

What & Why

  • Current: "When parameter got changed for multi-select combo box, the selected keys are not updated in UI... This PR added the missing logic to handle multi-select scenario."
  • Issue: None — description is clear and concise.
  • Recommendation: (Optional) You could include a one-line example of the broken behavior and the new behavior to make the change even clearer, e.g. "Previously: selected values remained as old display names; Now: selected displayNames update when options change for multi-select."

Impact of Change

  • Impact section is present and correctly indicates user-facing change. No major issues found.
  • Recommendation: Fill in the Developers/System bullets with brief notes (even if "none") to make the PR body more complete. Suggested text:
    • Users: Selected values in multi-select combobox now reflect updated option display names.
    • Developers: No API changes; internal state update for multiSelect behavior only.
    • System: No performance or dependency changes.

Test Plan

  • Unit tests were claimed and a new unit test was added in the diff (combobox.spec.tsx) that verifies multiSelect behavior on options update. Good.
  • E2E tests are not present — acceptable for this targeted UI logic change, but consider adding an E2E test if this component is critical in flows that CI covers.
  • Manual testing marked and "local" noted — fine.

⚠️ Contributors

  • The Contributors section is blank.
  • Recommendation: Add any reviewers/PMs/designers who contributed (if any) so they get credit. If none, leaving blank is fine.

⚠️ Screenshots/Videos

  • No visual assets included — acceptable for this small logic fix. If you expect UX reviewers to validate visuals, consider adding a screenshot showing the updated combobox state.

Summary Table

Section Status Recommendation
Title None needed
Commit Type None needed
Risk Level None needed
What & Why Optional: add a one-line before/after example
Impact of Change Add brief Developer/System notes (recommended)
Test Plan Consider E2E if component is critical
Contributors ⚠️ Add contributors or leave intentionally blank
Screenshots/Videos ⚠️ Optional: add if UX validation required

Final message:
This PR is good to go from a PR title/body compliance perspective. The title is clear, the commit type is correct, the risk label matches the body, and the unit test for the new multiSelect behavior is present in the diff. Please consider these small improvements before merging:

  • Add brief notes under Developers/System in the Impact section (even if just "none").
  • Optionally add an E2E test if this component is used in critical UI flows.
  • Add contributors if anyone else helped (for proper credit).
  • Optionally attach a screenshot showing the updated UI state if you want UX reviewers to validate visuals quickly.

Thank you — please update if you make additional changes and want a re-review of the PR body.


Last updated: Fri, 16 Jan 2026 06:14:22 GMT

@anthony0t anthony0t changed the title fix(ui): multi-select combox's selection isn't updated when options/key changed fix(ui): multi-select combobox selection not updated when options/keys change Jan 16, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where multi-select combobox components didn't update their displayed selections when the options changed (e.g., when display names were updated). The fix adds the missing setSelectedKeys call in the options-change effect handler, mirroring the existing logic for single-select mode.

Changes:

  • Fixed multi-select combobox selection state not updating when options change
  • Added missing dependency to useEffect dependency array
  • Added comprehensive unit test to verify the fix

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
libs/designer-ui/src/lib/combobox/index.tsx Added setSelectedKeys call when options change in multiSelect mode and included multiSelect in dependency array
libs/designer-ui/src/lib/combobox/test/combobox.spec.tsx Added test case to verify selectedKeys updates when options change in multiSelect mode

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@anthony0t anthony0t merged commit 1d7e872 into main Jan 16, 2026
23 checks passed
@anthony0t anthony0t deleted the tonytang/combobox branch January 16, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants