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
It's never used in another context but together with the
….Slider module as well as the ….init module. The ….init
module continues to require the ….Slider module, so all
dependencies are still met.
Motivated by the confusion about the two types of require()
introduced in Idf1cc79.
Bug: T233279
Change-Id: I4b4ef69f3074d57f884763c092a515ce976daaef
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: I7c98a41051e6d83ab3524cb14a709002feec2d78
It turns out the jQuery documentation is incomplete:
$( '<a>' ).offset() → { top: 0, left: 0 }
$( '<a>' ).parent().offset() → undefined
The difference is that the jQuery set in the second example is
empty.
Bug: T282067
Change-Id: I7c19162f1a39bd529e0a74a6cc0c1ac987f33657
… and replace them with more trivial `function …() {}`. I
believe this does not make any difference. But I feel this
makes the code a little more straightforward.
The motivation for this patch is because a few other patches
change some of these function declarations, leaving a (in my
opinion) confusing mixture of styles behind.
Change-Id: Ib8928c4176a963afcf1fee1c785dd7bdc86c9706
FIXME: Reusing HelpDialog as a module entrypoint creates a circular
reference. It's harmless because the dependencies are added at
different times, but also easy to refactor away.
Change-Id: I3608a78baddf2376cc9eb4524625f4911c130c06
In my PHPStorm IDE, this makes it possible to follow all methods and
properties in these classes, even these that are later defined.
Otherwise only the empty stub of each class is found.
This might be different in other IDEs.
Basically: PHPStorm does not understand the meaning of the $.extend()
syntax from jQuery without these hints.
Change-Id: I4aa76db183122f6669dc72561441f46f0056d793
This only solves part of the issue. Highlighting is applied for more
elements since they are retrieved from the DOM. The problem still exists
since highlighting depends on the RevisionList chunk on which is it applied.
To fix the issue completely highlighing should be managed somehow globally
on the global list.
Bug: T207781
Change-Id: Idda930f3d0dd64e767c68dade2ca8759bc636898
The next iterration on avoiding code duplication. I
still stuggle a bit merging these methods in a sane
way.
Change-Id: I8d95acc06da7f83a6133e55becabdf03b26a97af
- make use of toggelCSS()
- use more general jQuery selectors ro reset the line and bubble highlight
- get rid of ~= when selecting revision ids
Change-Id: I123e263bb379107a561fe8a2ffed476da9032b88
This is mainly a copy of what's done for the tag lines to set the events.
In the next steps I'll try to avoid code duplication here be extracting
common code.
Change-Id: I29df109de30c538cc206d445abff8464baf45378
The patch adds a first package of node selenium tests including
test for the user and tag filters.
The classes for user- and tag-rows were re-added to have better
access via selectors.
Change-Id: I8c53d9c923820e177d83ee900cee08e93cd3f65b
- move border to right so when the text starts there's a straight line
- increase right margin to fit mock
- adjust border radius so it's most probably always more than the line height
Bug: T218770
Change-Id: I76a2f096d14bf4d912686a71e5771ead1c7db3e6
This moves the retrieval of the revisions into the method setting the
highlighting. The check for undefined did not to much since the rev
var would still remain undefined.
Change-Id: I1acf540f135af4c16fb80d633b3690473ada7833
* Uses the existing implementation of highlighting revisions from the same user
* Shows bubble next to tags in Tooltip
* When you hover on a bubble, tag row is circled, and revisions with a specific tag gets highlighted. When hovering ends, highlighting ends. But, if there is an active filter available, all previous states are restored accordingly.
* When you click on a bubble, in addition to highlighting revisions and tag row, any previous tag or user filter is removed.
Bug: T203581
Change-Id: I824a027a7f542eb7227545870553e58ec23542bb
jQuery objects behave fine when they don't contain anything. No need
to check the .length before. This is one of the essential feature of
jQuery.
I'm also simplifying some setTimeout the same way. clearTimeout will
never fail. It will ignore numbers that never represented a timeout,
or have been cleared before. Note how the property was never set back
to -1. So this code was kind of incomplete anyway. I suggest to fix it
this way around, instead of trying to set -1.
Change-Id: Ic15812ead9d93f8eb07831aeb75577df2abdff07
Initial patch for introducing keyboard shortcuts reversed the blue and yellow lines. To fix that issue, the DOM order for pointer lines were put back to the original state in https://gerrit.wikimedia.org/r/#/c/473217/. Now, this patch tries to put the tabbing order in place reversing the changes made for the fix as per discussion in https://phabricator.wikimedia.org/T162119#4753481 and style changes.
Bug: T162119
Change-Id: Ic84503de0a877095c118abddb8066aeb667bc03c
This reverts parts of the patch I4d8db23 (T162119) that have
accidentally been done, as far as I can tell.
Note the revert in this patch here is untested and might partly break
the keyboard navigation feature introduced via T162119. However, I
strongly suggest to fix the bug T208238 first, and look back at
T162119 later, possibly via a newly created ticket.
Bug: T162119
Bug: T208238
Change-Id: I0c03e472b72ddd1414d3ca61970dba15158a8486
Also
- fix arrow width to one size of `20px` and
- make use of `.box-sizing` mixin in one occurrence.
Bug: T207905
Depends-on: I09aea78f8ebde37cf4f7de9bf0e14894c76ee722
Change-Id: Ibf4a6c8b77877a34668e91b0f5cf6c11a23e9f88
A free-standing, frameless button like this is better off with an
icon like 'helpNotice', part of OOUI since v0.28.2. It is a step
back to the original help icon appearance, the new one is still
available for example to use on toolbars.
Bug: T206753
Change-Id: I3d4102de2209e94fa732c1d1962c757fb7bca331
* Added `tabindex` on yellow and blue knobs to enable tabbing
* Used some of the logic of draggable functionality to support moving between revisions with keyboard shortcuts
* Also changed the source order of old and new pointer. Old pointer comes first in the DOM
Bug: T162119
Change-Id: I4d8db2352915c44aa11617edea5bcb0ac92ddc93
Completed subtasks
* Added bubble in the Tootltip
* Added border on clicking or hovering the bubble
* Highlites Bubble on hover
* When Clicked or hover on the Bubble: Highlights
revisions from the same user
* Added filter revisions.
Bug: T136105
Change-Id: I64cfef395ce1812d501980067edffe210fc99227
This is a direct follow up to the change done in I885f8e7. I found the
code very hard to read after the change, and tried to rearrange it a
little bit. This is what I came up with. What the code does is entirely
untouched.
This also rearranges the replaceWith calls so the replacements are done
from top to bottom.
This patch also avoids calling mw.config.set multiple times.
Change-Id: I1cbce8e8aa030d6ba5cd7d19ab26fa363e13474b
`$.fn.replaceWith` removes content from the DOM. Therefore, the
new content object was not being passed on by the hook.
Change-Id: I885f8e789b806a9ae09de1ed31a19a3f9db8144f
Approach followed:
- Include tags as an additional parameter in rvprop field to fetch revision tags
- Use API Tags to fetch available change tags for a wiki along with their display names
- At the time of fetching revision data rebuild the revision list with tags containing display names
- Display tags one in each line on the tooltip
Bug: T180429
Change-Id: Ieba8b79ed408ff50b3f7d4bcfd7b2fa8cca83278
Using rawElement function to generate a new html element for loading spinner.
This replaces the progress bar widget.
Bug: T161640
Change-Id: I949bce945def25de561c0ae0df971c3f609586ce
Added show() and hide() for mw-revslider-auto-expand-button in collapse and expand function.
Bug: T200263
Change-Id: Ic24a175dfe8d19853e32669f12b9f17e26730d6b
The 50 is the default. The worst-case scenario here is that users
beyond this limit don't have a gender for a while, until the next call
to the same API endpoint fetches the next 50 users with an unknown
gender.
Doing multiple API calls in advance is not worth it, in my opinion.
Bug: T197858
Change-Id: I0fdcc7ea96a6a5ee3934600c6f0fdc65263276e8
The arrow in Revision Slider flips on expand and collapse which is not according to the standard of OOUI.
Bug: T198626
Change-Id: I4205a2260e8507a09f2950566e5033bd58a74345
animation duration now has a factor that is dependent on
the square root of the animation distance.
Bug: T161883
Change-Id: Ic9fe89c7e3f984d3390260805e2480a0f8ffd0c9
including:
- Using LESS for nested CSS
- Overrule `margin` of frameless OOUI `mw-revslider-toggle-button`
- Removing CSS that had no effect since some time
- Removing `!important` where not longer needed
Change-Id: I1ba01061ebafe799ca62dbb6ce5e79459612af23
This patch mainly reintroduces the option to click on bars and move
the pointers with it. To do this, 'ghost' pointers are introduced
to show what would happen when bars are clicked. The pointers moved
differ depending on where the user clicks on a bar. Pointers are
still not allowed to change positions, so in some cases booth pointers
move with one click. See the task description.
The patch also includes some renaming and also refactoring of the
click handling in general. Furthermore bar hover mechanics are handled
by the RevisionListView class now.
Moving both pointers is not possible when it would push one of them of
limits.
Bug: T172092, T173566
Change-Id: I32a8256f7667e03081324d54accdf03a17454faf
Currently, when dragging a pointer over another revision but not
releasing it until it is at the same location, the diff still reloads
This change stores the initial positions and comparing at the end.
Bug: T163425
Change-Id: I430650d12bfeb3ce4af59abe91f18cc2b861a3de
Currently, `this` is passed as the parameter for pageName, making it
seem like getUrl() can take in an object as the first parameter, when
it should be a string. This works right now because of a tertiary
operator that fetches what we need. To reduce confusion, we should
pass null instead.
Change-Id: Ieea30a4933f3e3fdeb8331c5c544bf1eb034e6b7
of it appear normal. However, the revslider-tooltips are not inside
this globalWrapper, and thus need a separate CSS rule.
Bug: T166231
Change-Id: I2115688fee75145349e062b9586713a56541afa0
