Skip to content

Conversation

@Abhinegi2
Copy link
Member

@Abhinegi2 Abhinegi2 commented Dec 24, 2025

PR Checklist

Before you request a manual PR review from someone, please go through all of this list, check the points and mark them checked here.
This helps us reduce the number of review cycles and use the time of our core reviewers more effectively.

  • all automatic CI checks pass (🟢)
    • i.e. code formatting (ng lint) passes
    • i.e. unit tests (ng test) passes
    • in some cases unit test coverage can be difficult to achieve. If that or any other CI check fails, you can also leave a short comment here why this should be accepted anyway.
  • manually tested (all) required functionality described in the issue manually yourself on the final version of the code (developer)
  • reviewed the "Files changed" section of the PR briefly yourself to check for any unwanted changes
    • e.g. clean up debugging console.log statements, disabled tests, ...
    • please also avoid changes that are not directly related to the issue of the PR, even small code reformatings make the review process much more complex
  • 🚦 PR status is changed to "Ready for Review"
    • while you are still working on initial implementation, keep the PR in "Draft" status
    • once you are done with your initial work, change to "Ready for Review". This will trigger some additional automated checks and reviews.
    • (PR = "Ready for Review" does not immediately request a manual review yet, first complete the further checks below)
  • marked each code review comment as resolved OR commented on it with a question, if unsure
    • both implementing suggestions of automatic code review or discarding them as not applicable is okay
  • all checkboxes in this checklist are checked (to show the reviewer this really is ready)
  • 🚦 moved the issue related to this PR to Status "Functional Review" to request a personal review

⏪ if issues are commented from testing or code review, make changes. Please then re-check every item of the checklist here again.


Remaining TODOs:

  • unit tests

Summary by CodeRabbit

  • Refactor
    • Improved internal component initialization to better respond to input property changes.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

The component's initialization logic transitions from the OnInit lifecycle hook to the OnChanges hook. This enables the component to react to input property changes rather than executing only once during component initialization, while preserving the existing initValue() logic.

Changes

Cohort / File(s) Summary
Lifecycle Hook Migration
src/app/core/basic-datatypes/configurable-enum/display-configurable-enum/display-configurable-enum.component.ts
Replaced OnInit with OnChanges lifecycle interface. Renamed ngOnInit() to ngOnChanges(changes?: SimpleChanges), added override keyword, and inserted super.ngOnChanges(changes) call before existing initValue() invocation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • sleidig
  • sadaf895

Poem

🐰 From Init's single call, we leap with grace,
To Changes' watchful, responsive pace,
Dropdowns now dance when inputs shift their song,
Selection and filtering get along! 🎯✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title directly addresses the main issue being fixed: multi-select dropdown showing wrong options after selecting the first item, matching the linked issue #3526.
Linked Issues check ✅ Passed The code change implements the lifecycle hook migration from ngOnInit to ngOnChanges that was identified as the solution to fix the dropdown selection issue [#3526].
Out of Scope Changes check ✅ Passed The code change is focused solely on the DisplayConfigurableEnumComponent lifecycle hook migration, directly addressing the root cause identified in issue #3526 with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description follows the required template with all key sections completed and checkboxes marked as checked, indicating the author has completed the pre-submission checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

@Abhinegi2 Abhinegi2 self-assigned this Dec 24, 2025
@github-actions
Copy link
Contributor

Deployed to https://pr-3538.aam-digital.net/

@argos-ci
Copy link

argos-ci bot commented Dec 24, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Jan 8, 2026, 11:29 AM

@Abhinegi2
Copy link
Member Author

I tried to fix this issue, but it seems the root cause is related to cdk-virtual-scroll-viewport. I attempted several times:

  • Disabling trackBy during filtering
  • Forcing checkViewportSize() and scrollToIndex(0)
  • Creating new array references using [...array]
  • Increasing minBufferPx / maxBufferPx.
  • Using appendOnly mode

I also found the following open issues related to virtual scroll:

  1. Virtual Scroll template caching breaking behavior for Components with ngOnInit angular/components#16330
  2. bug(COMPONENT): CDK Virtual Scroller jump back/flickers to items on top angular/components#27104

but none of these approaches resolved our problem. then I checked how this works prroperly in single-select mode and noticed that the issue also exists there also
workaround for now may be to avoid using cdk-virtual-scroll-viewport for this dropdown. side effect of this approach could be performance issues with very large lists. but, since we already support filtering in the dropdown, it’s rare that users will scroll very long lists, so this trade-off might be acceptable.
@sleidig, what's your thought/opinion?

@sleidig
Copy link
Member

sleidig commented Jan 8, 2026

@Abhinegi2 , the issue you linked does seem to resolve it for me! Changed the internal enum display's ngOnInit to ngOnChanges, as suggested in angular/components#16330

@Abhinegi2 Abhinegi2 marked this pull request as ready for review January 8, 2026 11:26
@sleidig sleidig merged commit 7efa70d into master Jan 8, 2026
25 checks passed
@sleidig sleidig deleted the fix/dropdown-index-issue branch January 8, 2026 11:47
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.68.0-master.13 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released on @master managed by CI (semantic-release) label Jan 8, 2026
sleidig added a commit that referenced this pull request Jan 9, 2026
…arch text (#3538)

fixes #3526

---------

Co-authored-by: Sebastian Leidig <[email protected]>
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.68.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released managed by CI (semantic-release) label Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released on @master managed by CI (semantic-release) released managed by CI (semantic-release)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multi-select dropdown shows wrong options after selecting first item

3 participants