From 9a196ced0897d936eae442550520e6a6b6699a2e Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 30 Jan 2023 15:38:41 +0200 Subject: [PATCH] FEATURE: Move metadata user results to list bottom (#18977) Partial username or name matches were shown together with metadata matched results. This created a bad user experience because results that look unrelated were before even partial or exact group matches. --- .../app/components/composer-editor.js | 23 +++---- .../discourse/app/lib/user-search.js | 37 ++++++++--- .../templates/user-selector-autocomplete.hbr | 62 +++++++++---------- .../composer-editor-mentions-test.js | 37 +++++++++++ 4 files changed, 102 insertions(+), 57 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/composer-editor.js b/app/assets/javascripts/discourse/app/components/composer-editor.js index 7388a6fc3da..fb784007750 100644 --- a/app/assets/javascripts/discourse/app/components/composer-editor.js +++ b/app/assets/javascripts/discourse/app/components/composer-editor.js @@ -196,21 +196,6 @@ export default Component.extend(ComposerUploadUppy, { }; }, - @bind - _userSearchTerm(term) { - const topicId = this.get("topic.id"); - // maybe this is a brand new topic, so grab category from composer - const categoryId = - this.get("topic.category_id") || this.get("composer.categoryId"); - - return userSearch({ - term, - topicId, - categoryId, - includeGroups: true, - }); - }, - @bind _afterMentionComplete(value) { this.composer.set("reply", value); @@ -230,7 +215,13 @@ export default Component.extend(ComposerUploadUppy, { if (this.siteSettings.enable_mentions) { $input.autocomplete({ template: findRawTemplate("user-selector-autocomplete"), - dataSource: this._userSearchTerm, + dataSource: (term) => + userSearch({ + term, + topicId: this.topic?.id, + categoryId: this.topic?.category_id || this.composer?.categoryId, + includeGroups: true, + }), key: "@", transformComplete: (v) => v.username || v.name, afterComplete: this._afterMentionComplete, diff --git a/app/assets/javascripts/discourse/app/lib/user-search.js b/app/assets/javascripts/discourse/app/lib/user-search.js index 5c60c54e178..8dad5e38889 100644 --- a/app/assets/javascripts/discourse/app/lib/user-search.js +++ b/app/assets/javascripts/discourse/app/lib/user-search.js @@ -145,6 +145,10 @@ let debouncedSearch = function ( ); }; +function lowerCaseIncludes(string, term) { + return string && term && string.toLowerCase().includes(term.toLowerCase()); +} + function organizeResults(r, options) { if (r === CANCELLED_STATUS) { return r; @@ -152,39 +156,54 @@ function organizeResults(r, options) { const exclude = options.exclude || []; - const results = [], - users = [], + // Sometimes the term passed contains spaces, but the search is limited + // to the first word only. + const term = options.term?.trim()?.split(/\s/, 1)?.[0]; + + const users = [], emails = [], groups = []; + let resultsLength = 0; if (r.users) { r.users.forEach((user) => { - if (results.length < options.limit && !exclude.includes(user.username)) { - results.push(user); + if (resultsLength < options.limit && !exclude.includes(user.username)) { + user.isUser = true; + user.isMetadataMatch = + !lowerCaseIncludes(user.username, term) && + !lowerCaseIncludes(user.name, term); users.push(user); + resultsLength += 1; } }); } if (options.allowEmails && emailValid(options.term)) { - const result = { username: options.term }; - results.push(result); - emails.push(result); + emails.push({ username: options.term, isEmail: true }); + resultsLength += 1; } if (r.groups) { r.groups.forEach((group) => { if ( (options.term.toLowerCase() === group.name.toLowerCase() || - results.length < options.limit) && + resultsLength < options.limit) && !exclude.includes(group.name) ) { - results.push(group); + group.isGroup = true; groups.push(group); + resultsLength += 1; } }); } + const results = [ + ...users.filter((u) => !u.isMetadataMatch), + ...emails, + ...groups, + ...users.filter((u) => u.isMetadataMatch), + ]; + results.users = users; results.emails = emails; results.groups = groups; diff --git a/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr b/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr index 90db6c3907d..78c9c02ca2b 100644 --- a/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr +++ b/app/assets/javascripts/discourse/app/templates/user-selector-autocomplete.hbr @@ -1,47 +1,45 @@
diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js index 41f650c48ac..864915c9412 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js @@ -6,6 +6,7 @@ import { fakeTime, loggedInUser, query, + queryAll, } from "discourse/tests/helpers/qunit-helpers"; import { setCaretPosition } from "discourse/lib/utilities"; @@ -43,6 +44,17 @@ acceptance("Composer - editor mentions", function (needs) { avatar_template: "https://avatars.discourse.org/v3/letter/t/41988e/{size}.png", }, + { + username: "foo", + avatar_template: + "https://avatars.discourse.org/v3/letter/t/41988e/{size}.png", + }, + ], + groups: [ + { + name: "user_group", + full_name: "Group", + }, ], }); }); @@ -157,4 +169,29 @@ acceptance("Composer - editor mentions", function (needs) { "status expiration time is shown" ); }); + + test("metadata matches are moved to the end", async function (assert) { + await visit("/"); + await click("#create-topic"); + + await fillIn(".d-editor-input", "abc @"); + await triggerKeyEvent(".d-editor-input", "keyup", "@"); + await fillIn(".d-editor-input", "abc @u"); + await triggerKeyEvent(".d-editor-input", "keyup", "U"); + + assert.deepEqual( + [...queryAll(".ac-user .username")].map((e) => e.innerText), + ["user", "user2", "user_group", "foo"] + ); + + await fillIn(".d-editor-input", "abc @"); + await triggerKeyEvent(".d-editor-input", "keyup", "@"); + await fillIn(".d-editor-input", "abc @f"); + await triggerKeyEvent(".d-editor-input", "keyup", "F"); + + assert.deepEqual( + [...queryAll(".ac-user .username")].map((e) => e.innerText), + ["foo", "user_group", "user", "user2"] + ); + }); });