The method is only called from two places, and both do very similar
things before the call. This duplication can be merged.
One notable difference is that .scrollLeft() was possibly called
twice before. The first call was pointless anyway. The argument for
.scrollLeft() is absolute, not relative.
This code cleanup is motivated by T352169, but doesn't solve it.
Bug: T352169
Change-Id: I75e9ffc77ef6331f14e074921c78c28233e60840
This was almost the exact same code, with very few differences (e.g.
different CSS classes, and different icons).
Another change in the same file is the removal of an extra
`!== undefined` check. This is impossible. I can't tell why this was
there. Maybe an artifact from when this was developed?
This code cleanup is motivated by T352169, but doesn't solve it.
Bug: T352169
Change-Id: I3bb7ce00f9b754f9ba58310100b855c8ee3fca4a
Turns out we can run into this failure situation even if everything
goes according to plan. It is possible to click just one pixel outside
of the valid range, e.g. on the very right corner right next to the
last bar. This is supposed to not do anything anyway, and correctly
doesn't do anything from the user's perspective. But it shows up as a
failure in the JavaScript console.
Bug: T352169
Change-Id: I12c9cce90970be36667ba1b721afd38a13a063c9
When expanding the slider to newer or older revisions the handler
for revision clicks should only be added to the new revisions.
Change-Id: If590996d27dc75cbdfc931e9649418f875c3869d
This is a first step in restructuring and consolidating the
highlight and tooltip creation. I use revision focus to communicate
the state when a revision is highlighted and shows a tooltip.
There's still stuff to untangle but I want to keep the diffs small.
Change-Id: I0b169042837a2c3bb825c23368e7e8a485694eb5
See T349208 for an explanation. It looks like the SliderView.render
function was written with the assumption that it's only triggered
once on construction time. But since T139101 it's triggered again
for every window resize event. This adds the same event handlers
over and over again to existing elements that aren't affected by
the SliderView.render function.
This will probably become even worse with I49878fd (T336729).
Please test this carefully. I'm not 100% sure this is the best
possible fix.
Bug: T349208
Change-Id: Iba22924b660f2709c0680aa6fbeb0feba92cfa76
There's no need and probably just minimal gain to use these methods
with a revision as input. We could always just cleanup all tooltips
or wrapper highlighting before we set a new one. Makes the code
much simpler.
Change-Id: I34594843ccafa83372c796ff8cca68c4d6b58e06
Adds two handlers to make sure the popup is closed when the focus
moves away from either the pointers or contents of the a popup.
Bug: T341874
Change-Id: Ia68fc5ffbb63b4a534c84987879499e06cd60238
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
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
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
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
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
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
