Skip to content

Conversation

@tyb-talks
Copy link
Contributor

@tyb-talks tyb-talks commented Sep 23, 2025

This replaces the jquery-based autocomplete in user-selector with the DMultiSelect component (similar to what was done in discourse/discourse#34685).

Existing behaviour:

text-field-user-selector.mov

Caveat:
I'm unsure how the higher level wizard components depend on the user-selector being a TextField-based component.. If we can't use DMultiSelect here, the alternative would be applying the DAutocomplete modifier directly to the text field (example: discourse/discourse#34131).

Key Changes

  • Converted custom-user-selector from Classic Ember component to Glimmer component, passing in required properties explicitly from parent Classic component

  • Replaced jQuery autocomplete with DMultiSelect for modern UI patterns

  • Maintained backward compatibility with existing @onchangecallback interface

  • Automatic user exclusion: DMultiSelect automatically filters selected users from search results (eliminates
    custom excludedUsernames() logic)

  • Built-in keyboard navigation and accessibility features

  • Search results display: Users show avatars + username/name, groups show group icon + name

  • Selected items display: Plain text (username for users, name for groups) without icons - matches original
    behavior

  • Modern dropdown positioning using FloatKit with configurable placement options

  • Removed Legacy Options

    • allowAny: Not needed with DMultiSelect (was jQuery autocomplete-specific)
    • updateData: Replaced by DMultiSelect's automatic reactive updates
    • Template functions: Replaced with Handlebars named blocks (<:selection>, <:result>)

Testing

New behaviour:

d-multi-select-user-selector.mov
@tyb-talks tyb-talks force-pushed the dev-refactor-custom-user-selector-autocomplete branch from be02794 to e6e6286 Compare September 23, 2025 10:56

actions: {
updateFieldValue(usernames) {
this.set("field.value", usernames);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is meant to do the same thing as the previous 2-way binding from TextField and assumes there's some kind of dependency on this.field.value for this wrapper component. I may be wrong here and this is not necessary.

Copy link
Member

@angusmcleod angusmcleod left a comment

Choose a reason for hiding this comment

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

@tyb-talks Amazing, thank you.

@tyb-talks
Copy link
Contributor Author

@tyb-talks Amazing, thank you.

🙇

Could I also trouble you to trigger the CI? (I don't see an option for forcing a run and it seems stuck)

And I don't really have the context to do a comprehensive manual QA on the wizard, if you have something scaffolded up on a test site or locally, it would be safer to do a check on your end. The plugin's test suite fails on a few tests locally which seem unrelated to this change so I suspect my local setup isn't fully compatible with this plugin.

@tyb-talks
Copy link
Contributor Author

@tyb-talks Amazing, thank you.

🙇

Could I also trouble you to trigger the CI? (I don't see an option for forcing a run and it seems stuck)

And I don't really have the context to do a comprehensive manual QA on the wizard, if you have something scaffolded up on a test site or locally, it would be safer to do a check on your end. The plugin's test suite fails on a few tests locally which seem unrelated to this change so I suspect my local setup isn't fully compatible with this plugin.

@angusmcleod in case you missed the above :)

@angusmcleod angusmcleod merged commit e9adaac into paviliondev:main Oct 20, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants