Resolve TagIt's keyword separator character interference (Arabic/Cyrillic)

[asmecher]

Tag-it has several problems...

• Keyword separator character interferes with Arabic and Cyrillic
• It may not be compatible with JQuery 3.x: see comments at Review dependency management #3404

Evaluate possible replacements, choose one, and implement!

[asmecher]

(https://select2.org/ seems popular...)

[NateWr]

Select2 is popular but my understanding is that it has accessibility issues. I doubt Tag-it is any better, though. Eventually we will phase out Tag-it as forms move to Vue.js, where we'll use an autosuggest that attempts to be WCAG compliant.

[ajnyga]

This #1550 is very much connected to this issue.

[jamshidhashimi]

If accessibility is an issue, I was not able to find a library that completely addresses it. For example, the libraries and associated issue: Select2, tag-it, SelectWoo and others.

awesomplete is nice and lightweight, I couldn't find any open issues on accessibility in its repo but doesn't solve everything we want, at least doesn't have the UI we are used to in tag-it and select2.

If we are moving to Vue anyways, we can wait for it. To address some other issues like this and this in tag-it, there is a hack around it that we can implement before we move to Vue.js. Although select2 has a built-in solution for it.

Any further thoughts?

[NateWr]

Personally, I am tempted to just wait until we make it obsolete with the Vue.js forms. But if you or someone wants to do a quick swap in the meantime to something better I have no objections.

It would also be possible to turn each tag-it field into a little Vue.js app that loads https://github.com/Educents/vue-autosuggest. But, again, we might be bending over backwards for something that will be replaced before too long.

[asmecher]

@NateWr, I think tag-it is currently broken since the JQueryUI upgrade, no?

[NateWr]

Oh I didn't realize! In that case, we do need to replace it. 😂

@jamshidhashimi I'd say choose the one with the API you like the best and bring it in, ideally if it can be integrated through the package.json or composer.json files, so that we can more easily keep it updated.

[jamshidhashimi]

I learnt from here that this fork works with jQuery 3. From there, I further learnt that .andSelf() is depreciated in jQuery 3 and that should be changed to andBack() and that seems to be the source of the issue.

Should I go with the fix or replace the library? sorry, just wanted to make sure :)

[ajnyga]

+1 for fixing this in tag-it, if in the long it will be replaced anyway (thinking of myself of course since I have a plugin that hooks into tag-it autosuggest...)

[jamshidhashimi]

On compatibility with JQuery 3.x:
Going through the issues and merged PRs, I noticed @asmecher already applied the patch I mentioned above. So there shouldn't be an issue regarding that.

On resolving the issues available:
On resolving this issue on Arabic and this issue on Cyrillic, I found out that the source of the issue is KeyboardEvent.keyCode (in addition to being deprecated), because although the KeyboardEvent.key changes, KeyboardEvent.keyCode is always 188, in any language keyboard. You can test the behaviour here. To resolve the issues we are dealing with in the scope of this and related issues, changing the line:
(event.which === $.ui.keyCode.COMMA && event.shiftKey === false) ||
to
(event.key === ',' && event.shiftKey === false) ||
does the work. This keeps the comma (,) working fine in any keyboard that uses it and stops thinking that (و) in Arabic or Farsi and (б) in Cyrillic is a comma. I also tested the solution with Turkish keyboard, where the key in the keyboard that generates comma is located in a different place than the English one and it works fine.

On supporting commas in all languages:
I also thought of providing support for the Arabic and Cyrillic comma through tag-it, but I noticed there are a lot of types of commas out there. It looks possible to provide that support, as we can detect ctrl, shift and meta keys through JavaScript and combining that with the event.code can let us fire actions. This solution needs a complete list of keyboard shortcuts to incorporate the solution.

Any thoughts?

[NateWr]

I would say go ahead and make the change in the tag-it library for keyCode, as a temporary stop-gap until we migrate away from it.

there are a lot of types of commas out there.

Yeah, let's avoid trying to maintain something like that ourselves. We'll address issues as they arise.

[NateWr]

(ps - great research and thanks for the explanation!)

[jamshidhashimi]

PR:
#4285

@asmecher @NateWr please review. Thanks.

[asmecher]

@NateWr, would you mind reviewing this one? Thanks!

[NateWr]

@jamshidhashimi It looks like you've closed that PR. Is there an active one you want me to look at?

[jamshidhashimi]

Sorry for that @NateWr. Submitted a new PR.

PR:
#4302

[NateWr]

Looks good @jamshidhashimi. I'm happy to merge this. Can you set up an OJS pull request just to run the tests? I just want to make sure we don't break anything there.

[jamshidhashimi]

@NateWr submitted the pkp/ojs#2224 for test in OJS. All tests are have passed.

[NateWr]

Merged! Thanks @jamshidhashimi.