Skip to content
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.

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

Angular Select placeholder integration #1912

Merged
merged 84 commits into from
Mar 12, 2024
Merged

Conversation

atmgrifter00
Copy link
Contributor

@atmgrifter00 atmgrifter00 commented Mar 7, 2024

Pull Request

🤨 Rationale

👩‍💻 Implementation

  1. Expose the necessary Angular attributes for the NimbleListOptionDirection to support the placeholder use-case
  2. Changes to nimble-components to address a problem discovered during early integration attempts for a particular SLE usage.
    • It was discovered that there are scenarios that will add nimble-list-options dynamically to the Angular template contents, followed immediately with setting a form value (that the Select was bound to) to the dynamically added option's value. This results in situation where the SelectControlValueAccessor processes the set value (which for the Select requires that option to be present on the web component side) prior to the Select adding the option to its set, which would result in the Select view ignoring that value and setting it to empty string instead.
    • The solution to this is to leverage the connectedCallback override of the NimbleListOption to allow it to have its parent, through an available interface if implemented, process it. All the Select does, is if the option isn't already part of its options, it simply pushes it on to it. There is no concern with this interfering with the normal process of the option being added as part of the slottedOptionsChanged callback, that will simply result in overwriting the options (which is fine).
    • Applied this solution to the combobox too
  3. Exported the SelectPageObject from Angular, so that we can also leverage it in SLE as needed. Removed a lot of unnecessary async aspects of the SelectPageObject public methods.
  4. Updated example app to use the feature and removed extra items from select to align with Blazor app and avoid Lighthouse performance degradation.

🧪 Testing

Added unit tests in nimble-components for the new internal interface the Select is using. Added the boilerplate unit tests in Angular for the new attributes. Also added a couple of Angular tests one of which is representative of the original failing scenario from the integration effort (and the other is more for completion).

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@atmgrifter00 atmgrifter00 requested review from jattasNI and rajsite March 8, 2024 22:04
@jattasNI jattasNI requested a review from rajsite March 12, 2024 20:12
@rajsite rajsite enabled auto-merge (squash) March 12, 2024 21:44
@rajsite rajsite merged commit 880dfa1 into main Mar 12, 2024
10 checks passed
@rajsite rajsite deleted the select-placeholder-Angular branch March 12, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants