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

Search people's names modulo diacritics, in autocomplete #3710

Closed
maxandersen opened this issue Dec 5, 2019 · 5 comments · Fixed by #5292
Closed

Search people's names modulo diacritics, in autocomplete #3710

maxandersen opened this issue Dec 5, 2019 · 5 comments · Fixed by #5292
Assignees
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending @zulip/shared Good spots to share code with the webapp

Comments

@maxandersen
Copy link

autocomplete seem to not work consistently for characters like àåä

If someone is named ståhle I can't get that to autocomplete it using @sta
but if you are named stéph autocomplete works with @ste

@gnprice
Copy link
Member

gnprice commented Dec 6, 2019

Thanks @maxandersen ! I agree this would be a useful feature.

It looks like the webapp added this feature earlier this year, in zulip/zulip#11183 / zulip/zulip@b7a0a45f0. Before that, it had similar logic which it used for searching for a user in the right sidebar; we probably also want to use similar logic when searching for a user to e.g. start a PM thread.

A note on how to implement this: this looks like an example of some logic it'd be great to share the code for between web and mobile. For whoever takes this on, see

  • c631bdb and preceding commits in src/users/usersActions.js for an example of using code shared from the webapp
  • zulip/zulip@a63786a for an example of moving webapp code that wasn't previously shared into the shared package, and zulip/zulip@b70b7df for adding Flow types on it
  • docs/howto/yarn-link.md for some mechanics for efficiently developing changes in shared code

In particular:

  • remove_diacritics should definitely be shared, and I think it will be easy to share.
  • person_matches_query would probably be good to share. It may take some more refactoring work to share, though. So that part should come after fixing this issue.

@gnprice gnprice changed the title make autocomplete work on their base characters Search people's names modulo diacritics, in autocomplete Dec 6, 2019
@gnprice gnprice added a-compose/send Compose box, autocomplete, camera/upload, outbox, sending @zulip/shared Good spots to share code with the webapp labels Dec 6, 2019
@rk-for-zulip
Copy link
Contributor

rk-for-zulip commented Dec 11, 2019

remove_diacritics is greatly simplifiable if we have access to String.prototype.normalize:

export const remove_diacritics = (str: string): string => 
  str.normalize("NFD")
    .replace(/[\u0300-\u036F\u1AB0-\u1AFF\u20D0-\u20FF\uFE20-\uFE2F]/g,'');

(This isn't quite complete; it will notably not remove strokes, such as those found in Ħ, Ł, Đ, or Ð [sic!], nor unify i with Turkish ı. Also, we may or may not want it to unify homographs.)

@timabbott
Copy link
Member

zulip/zulip#13485 is an improved implementation we just merged for the webapp for this.

@gnprice
Copy link
Member

gnprice commented Jan 1, 2020

Neat!

Here's the new version there, which is in a very similar spirit to Ray's comment above:

const unicode_marks = /\p{M}/gu;

exports.remove_diacritics = function (s) {
    return s.normalize("NFKD").replace(unicode_marks, "");
};

At that length it'd feel less bad to me to duplicate it. ... But also that change itself illustrates why it'd be better to share it. (The possibility we might add more normalization in the future -- so e.g. "Luk" finds "Łukasz", following on an example above -- is another reason.) Since it should be an easy one to share, I think that'll still be the preferred way to solve this issue.

@showell
Copy link

showell commented Jan 30, 2020

remove_diacritics is now in static/shared/js/typeahead.js.

@Fingel Fingel self-assigned this Mar 11, 2022
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 11, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 11, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 14, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 15, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 15, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Fingel added a commit to Fingel/zulip-mobile that referenced this issue Mar 16, 2022
When invoking the typeahead filter for users it is desirable to be able
to see results for users with diacritics in their names even if the
filter does not contain the matching diacritics. This PR leverages
@zulip/shared typeahead module in order to strip diacritics out of the
filtering process, unless the user explicitly uses them in the filter.

Fixes: zulip#3710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-compose/send Compose box, autocomplete, camera/upload, outbox, sending @zulip/shared Good spots to share code with the webapp
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants