23 Feb
2018
23 Feb
'18
7:24 a.m.
Hi devs,
In October / November of last year, we integrated the Keypress JS
library as a replacement of OpenJS Keyboard in order to better handle
keyboard shortcuts in the platform and its extensions and add new
shortcut types (such as "sequence" shortcuts) useful for [1].
Moving to this library has also brought some bugs, such as XWIKI-15024
[2]. Currently, that bug can be fixed upstream by the library
maintainer, that unfortunately isn't very active on the project.
Choosing a library that isn't properly maintained might have been a
problem that we didn't really thought of when switching to Keypress JS.
In order not to be stuck by waiting for upstream releases, I propose
that we fork the library, and fix the problem raised by XWIKI-15024
ourselves by reviewing and integrating a patch proposed upstream, but
not yet merged [3]. Of course this solution has pros and cons, the main
downside being that we will need to maintain our fork.
Also note that, if we fork the project, we will have to choose either to
keep the source code in its original form (which is CoffeeScript [4]) or
to translate the code into JavaScript, which is more maintainable for
us, but that will probably not allow us to integrate other patches from
the official project branch.
WDYT ?
Thanks,
Clément
[1]
http://www.xwiki.org/xwiki/bin/view/Documentation/UserGuide/Features/Keyboa…
[2] https://jira.xwiki.org/browse/XWIKI-15024
[3] https://github.com/dmauro/Keypress/pull/137
[4] http://coffeescript.org/
23 Feb
23 Feb
8:20 a.m.
Hi Clement,
On 23 Feb 2018, at 07:24, Clément Aubin
<aubincleme(a)gmail.com> wrote:
Hi devs,
In October / November of last year, we integrated the Keypress JS
library as a replacement of OpenJS Keyboard in order to better handle
keyboard shortcuts in the platform and its extensions and add new
shortcut types (such as "sequence" shortcuts) useful for [1].
Moving to this library has also brought some bugs, such as XWIKI-15024
[2]. Currently, that bug can be fixed upstream by the library
maintainer, that unfortunately isn't very active on the project.
Choosing a library that isn't properly maintained might have been a
problem that we didn't really thought of when switching to Keypress JS.
In order not to be stuck by waiting for upstream releases, I propose
that we fork the library, and fix the problem raised by XWIKI-15024
ourselves by reviewing and integrating a patch proposed upstream, but
not yet merged [3]. Of course this solution has pros and cons, the main
downside being that we will need to maintain our fork.
Also note that, if we fork the project, we will have to choose either to
keep the source code in its original form (which is CoffeeScript [4]) or
to translate the code into JavaScript, which is more maintainable for
us, but that will probably not allow us to integrate other patches from
the official project branch.
WDYT ?
+1 to do a quick fork to fix XWIKI-15024 because it’s a blocker so we need to do whatever
it takes to fix ASAP (ASAP = in 1 or 2 days).
-1 to keep this fork. This is just painful and costly. It means we’ve picked the wrong JS
library for shortcuts and that we’ve picked an unmaintained one. Bad choice :( Thus I’d
recommend to review existing JS libraries more to try to find a better one (we don’t
necessarily need all the bells and whistles of KeypressJS, we just need to be able to add
our developer shortcuts). Imagine if we had to do this for every library we pick :) That’s
why we need to be careful to only pick maintained libs and move away from them when
they’re not anymore. It’s definitely not the objective of XWiki to maintain JS code for
supporting shortcuts (if we can avoid it ofc, meaning that if there aren’t any libs out
there we won’t have the choice but we need to search again for one first).
-1 to convert to JS for now. We need to bring the minimal modification in the hope that
the PR we submit to Keypress JS gets reintegrated.
Thanks
-Vincent
10:37 a.m.
On Fri, Feb 23, 2018 at 8:20 AM, Vincent Massol <vincent(a)massol.net> wrote:
Hi Clement,
Same logic as Vincent. Big -1 to end up maintaining a fork of keypress
forever. It's really not critical enough for us.
On 23 Feb 2018, at 07:24, Clément Aubin
<aubincleme(a)gmail.com> wrote:
Hi devs,
In October / November of last year, we integrated the Keypress JS
library as a replacement of OpenJS Keyboard in order to better handle
keyboard shortcuts in the platform and its extensions and add new
shortcut types (such as "sequence" shortcuts) useful for [1].
Moving to this library has also brought some bugs, such as XWIKI-15024
[2]. Currently, that bug can be fixed upstream by the library
maintainer, that unfortunately isn't very active on the project.
Choosing a library that isn't properly maintained might have been a
problem that we didn't really thought of when switching to Keypress JS.
In order not to be stuck by waiting for upstream releases, I propose
that we fork the library, and fix the problem raised by XWIKI-15024
ourselves by reviewing and integrating a patch proposed upstream, but
not yet merged [3]. Of course this solution has pros and cons, the main
downside being that we will need to maintain our fork.
Also note that, if we fork the project, we will have to choose either to
keep the source code in its original form (which is CoffeeScript [4]) or
to translate the code into JavaScript, which is more maintainable for
us, but that will probably not allow us to integrate other patches from
the official project branch.
WDYT ?
+1 to do a quick fork to fix XWIKI-15024 because it’s a blocker so we need to do whatever
it takes to fix ASAP (ASAP = in 1 or 2 days).
-1 to keep this fork. This is just painful and costly. It means we’ve picked the wrong JS
library for shortcuts and that we’ve picked an unmaintained one. Bad choice :( Thus I’d
recommend to review existing JS libraries more to try to find a better one (we don’t
necessarily need all the bells and whistles of KeypressJS, we just need to be able to add
our developer shortcuts). Imagine if we had to do this for every library we pick :) That’s
why we need to be careful to only pick maintained libs and move away from them when
they’re not anymore. It’s definitely not the objective of XWiki to maintain JS code for
supporting shortcuts (if we can avoid it ofc, meaning that if there aren’t any libs out
there we won’t have the choice but we need to search again for one first).
-1 to convert to JS for now. We need to bring the minimal modification in the hope that
the PR we submit to Keypress JS gets reintegrated.
Thanks
-Vincent
--
Thomas Mortagne
11:29 a.m.
On 02/23/2018 10:37 AM, Thomas Mortagne wrote:
[1]
https://github.com/xwiki/xwiki-platform/blob/e8ad6b9ac70d7517eabc266a37a000…
[2] https://paste.ubuntu.com/p/DFx5NxBcmP/
[3] https://github.com/ccampbell/mousetrap
On Fri, Feb 23, 2018 at 8:20 AM, Vincent Massol
<vincent(a)massol.net> wrote:
I get that maintaining yet another fork is a pain, that said, that's
what we did when we were using OpenJS Keyboard [1], in a limited manner.
Now, if we want to focus on solving our problem, which is in definitive
fixing XWIKI-15024 but also providing an API that works for
unregistering shortcuts, we can indeed look at the alternative libraries
to Keypress JS.
[2] presents the results of some researches for alternative libraries ;
AFAICS, there are not that much libraries that are both maintained and
offering support for our use case. The best alternative that we could
have to Keypress JS is Mousetrap [3] which seems to provide the same
functions but isn't really accepting more PRs than Keypress JS.
Moreover, moving to Mousetrap would involve rewriting the interface that
is used in the platform for registering shortcuts for being compatible
with the new lib, which might not take "1 or 2 days".
Hi Clement,
Same logic as Vincent. Big -1 to end up maintaining a fork of keypress
forever. It's really not critical enough for us. On 23 Feb 2018, at 07:24, Clément Aubin
<aubincleme(a)gmail.com> wrote:
Hi devs,
In October / November of last year, we integrated the Keypress JS
library as a replacement of OpenJS Keyboard in order to better handle
keyboard shortcuts in the platform and its extensions and add new
shortcut types (such as "sequence" shortcuts) useful for [1].
Moving to this library has also brought some bugs, such as XWIKI-15024
[2]. Currently, that bug can be fixed upstream by the library
maintainer, that unfortunately isn't very active on the project.
Choosing a library that isn't properly maintained might have been a
problem that we didn't really thought of when switching to Keypress JS.
In order not to be stuck by waiting for upstream releases, I propose
that we fork the library, and fix the problem raised by XWIKI-15024
ourselves by reviewing and integrating a patch proposed upstream, but
not yet merged [3]. Of course this solution has pros and cons, the main
downside being that we will need to maintain our fork.
Also note that, if we fork the project, we will have to choose either to
keep the source code in its original form (which is CoffeeScript [4]) or
to translate the code into JavaScript, which is more maintainable for
us, but that will probably not allow us to integrate other patches from
the official project branch.
WDYT ?
+1 to do a quick fork to fix XWIKI-15024 because it’s a blocker so we need to do whatever
it takes to fix ASAP (ASAP = in 1 or 2 days).
-1 to keep this fork. This is just painful and costly. It means we’ve picked the wrong JS
library for shortcuts and that we’ve picked an unmaintained one. Bad choice :( Thus I’d
recommend to review existing JS libraries more to try to find a better one (we don’t
necessarily need all the bells and whistles of KeypressJS, we just need to be able to add
our developer shortcuts). Imagine if we had to do this for every library we pick :) That’s
why we need to be careful to only pick maintained libs and move away from them when
they’re not anymore. It’s definitely not the objective of XWiki to maintain JS code for
supporting shortcuts (if we can avoid it ofc, meaning that if there aren’t any libs out
there we won’t have the choice but we need to search again for one first).
-1 to convert to JS for now. We need to bring the minimal modification in the hope that
the PR we submit to Keypress JS gets reintegrated.
Thanks
-Vincent
2397
days inactive
2397
days old
3 comments
3 participants
participants (3)
-
Clément Aubin -
Thomas Mortagne -
Vincent Massol