The pointer click and revision click handlers did almost the same
and the former was able to handle events from the latter. I merged
them and could then shortcut some code.
Change-Id: Id224b8d8da653110134cce0385da3a18cd073ecf
I'm not sure why it was done this way. Probably because it doesn't
make an actual difference from the user's perspective. My motivation
is: When we already called the code that auto-expands the
RevisionSlider UI then it doesn't make much sense to give the user
a keyypress handler that does the same a second time.
Possibly even related to T342556?
This patch also contains a few small, unrelated code cleanups.
Change-Id: I123e89d9d7dc3b1e33cf43831c679330d9dd1cdd
Turns out this code can only ever access a single underline: The one
it owns and can access via it's own this.$html property. One of the
two jQuery selectors always turned out empty, and calling .css() or
anything else on an empty jQuery collection just doesn't do anything.
This patch also contains a similar, but technically unrelated
cleanup in the init code.
Change-Id: Ic89b11971f51f5dcca67dcbd308f65310f48f0ec
Two almost identical pieces of code, both adding revisions to the
list.
There is a 3rd one that prepends revisions. I leave this untouched
for now.
Change-Id: I4798c8c2d6e0f9b70f7ea0dc20bb271514c03350
When the user uses the keyboard to interact with the slider, the
revisions can changed by moving the pointers with the arrorw keys.
In that case the pointers have keyboard focus. To allow tabbing
into the popup from that position, the tooltip needs to follow the
pointer in the DOM. That's what's done in this patch.
Bug: T341872
Change-Id: I75ef7c32fb105526552eac387ff5a5bda8eefe1b
It took me a while to come up with a solution for this, but there
are several things that seem to trigger the showTooltip method a
bit to often.
Tooltips can be triggered either by the handlers that deal with
mouseenter and mouseleave events that also trigger revision high-
lighling. But tooltips can also be triggered when you use the
keyboard or the mouse while dragging sliders.
The mechanics on what's highlighted and what's triggering a popup
are a bit weiridly setup and there could probably be a major re-
factoring done to make things clear ( for example the show popup
method also highlights the revision but the highlight revision
mehtod does not ). I had a quick approach to fix that, but it's
not too easy.
Another issue is, that some events fire off a delayed popup close
mechanism. So the solution here reads like:
If you are triggered to show a popup, and that popup exists
already, stop the delayed popup close mechanism and bail out.
Bug: T341872
Change-Id: I2646a69cccd549af902d57fdf4ff6fb0e94cbe64
The plain wikitext comment is apparently not used for anything,
anywhere. It was for some reason used for an "is empty" check. But
we can do the same with the `parsedcomment`. I checked and an empty
comment doesn't result in something non-empty like `<div></div>`, but
stays as an empty string.
Change-Id: Iedc5898b7b0f82231328ab3e0e46b1461ca845b1
This code was designed for the left/right cursor keys. But it
currently triggers on all keys. This causes confusing behavior when
tabbing through the UI. This code also triggers on tab/shift+tab,
which can be visibly seen when the wrong popup opens. It also
triggers on enter, which feels like it's intentional, but is nore a
happy accident.
Especially note how the buildTabbingRulesOnKeyUp handler directly
below does the exact same: It only acts on left/right, but no other
key.
We intentionally keep the existing (even if bogus) behavior for the
enter key. To be replaced with something better in a later patch.
Bug: T341874
Change-Id: I75aac4ea3a66a69a44756159c8a98acdc6e74b01
Reintroduces IIFE closures in test files because variables were
declared in the global namespace, and "const" now causes hard errors.
Bug: T339323
Change-Id: I69e9d7a29591137f185f3e5ab02dea590ec4dff6
I'm not sure why it was done like this. It looks like we can skip
both the extra conditional as well as the extra function scope.
Change-Id: I9aebd17bece0b9a573fc1f9697e79b759741751e
This is certainly not an actual fix for anything. But it makes any
resize issue look much less broken and much more bearable. I think it
is helpful to have this extra safety in place even after we properly
fixed the issue.
Bug: T336729
Change-Id: I724ede9cda120f18c4b7ee3ebf7bb41c0541819e
According to https://api.jquery.com/html/ a jQuery object is not
supported in .html() although is works.
The .empty() is needed to avoid multiple sliders on resize.
Change-Id: I0ce4748e95529dbe27f82d6fd0aa2433bfda4375
Mostly motivated by the missing @type tag.
Also make the syntax more compact. This is significantly easier to
read, I would like to argue.
Change-Id: If5cea5ff66ed345df16a2b417f0db4c56db347c9
This might not be the best possible solution, but it improves the
current, obviously broken situation a lot. At the moment one of the
dots is drawn outside of the slider, even if the revision it should
point at is part of the slider. Turns out the revisions shown on the
slider are loaded in multiple steps. The first step misses one of the
revisions when their order is swapped. When the missing revision is
added later it's already to late.
Bug: T168609
Change-Id: I10d15d04d981c87d35b2431080182fb5e3eb2b2b
Removed the HTML in the tutorial message and used linebreaks(\n\n) in i18n/en.json file,
replaced .html( mw.message( '…' ).…() ) with .text( mw.msg( '…' ) ) in modules/ext.RevisionSlider.HelpDialog.js
and added 'white-space: pre-wrap' in class '.mw-revslider-column-text' in file modules/ext.RevisionSlider.less
without affecting the front end of the page.
Bug: T267128
Change-Id: Iaf8b1819ca3139c18fb65cfca1c0b2120abb10f4
The new mw eslint config comes with node 12 and the change will be
quite big due to the lock file. I wanted to keep the diff of actual
code changes seperate.
- Applied all code style recommendations
- Removed one test that's not giving any value
- Changed regex .match to .test for performance and convinience
Change-Id: I578be8c6460c7a4d1220354c028a9bfd9bb86d13
It's never used in another context but together with the
….Slider module.
Motivated by the confusion about the two types of require()
introduced in Idf1cc79.
Bug: T233279
Change-Id: I922d7ab56fd3ce80bc901f1a5d7174df6fe6756d
