Elements that are marked as collapsible (often tables, but can
actually be anything) are most certainly marked as such because
they are big and don't fit in a popup.
Another plugin makes tables sortable.
In both cases non-functional UI elements appear in the popup.
We decided:
* Hide collapsible elements (no matter if currently collapsed
or not), and show a placeholder text instead.
* Remove sortable arrows.
This is a baseline patch that solves everything, except the
(i) icon is missing. This will be added in the next patch.
Bug: T220208
Change-Id: I58f3036bf4988d0ebe5716b0a54506446fca10c3
There are 3 types of links:
* Links with no class are links to other pages in the same wiki.
* Links with "extiw" are interlanguage as well as interwiki
links to other known wikis (e.g. from Wikipedia to Wikidata).
* Links with "external" point to somewhere else on the web.
Bug: T215419
Bug: T239230
Change-Id: Ia25db94d02670a919fc003f3e3562725de2e8eae
We found that Popups was overcounted approximately 2.5x relative to Cite.
This patch attempts to nearly match the circumstances under which Cite
(and its tracking) is loaded.
Bug: T214493
Change-Id: Ib31df3c33879f4ea63d9808ffd260861069a8977
The Popups pageviews were suspiciously high, about 2.5x higher than
Cite counts which should have reported the same number. This patch
attempts to deduplicate pageview events.
Bug: T214493
Change-Id: I51bf6d1909d65ecd9d3ef28eda285852897343ec
We want to know the side of this population in pageviews, in order to
produce proportional metrics later.
Bug: T214493
Change-Id: I940872870754e85d19688f3855d6857e65b982b1
We're getting a few dozen records per day at 1:10, so let's stop sampling.
We need enough data to get significance on events happening at 0.1% or less.
Bug: T214493
Change-Id: I8a913fe1ee1e5b72d84914e183ac2386ddb20d84
Notice how this actually reduces the size of the final, compiled index.js.
It's not much, but still.
One issue I noticed is that the coverage reports for the JS code stopped
working. I have no idea why.
Bug: T208951
Change-Id: I2fe92579574b3b1ba4d2dd064899eee944045a96
We've logged zero events after 12hr of deployment, so it should be
safe to increase sampling by 100x.
Bug: T214493
Change-Id: Icd67aed448269f603dd4465f7e46eac9a64bd1ab
Logs events to the ReferencePreviewsPopups EventLogging schema, in
order to understand whether Reference Previews is helpful for
end-users.
This will be removed along with the older tracking, as soon as our analysis
phase is finished.
Incidentally disables a lint rule for the generated JS, it's about
readability so irrelevant to the minified code.
Bug: T214493
Change-Id: I2638611ba67b785338f7e98a1c4b08a5e829812d
We did a strict === comparison before. This works fine when the page
zoom is either 100% or 200%. Other zoom factors produce fractional
numbers. Comparing them with === is doomed to fail because these numbers
are IEEE representations, not necessarily being identical in situations
where you would expect them to be identical.
This applies two fixes: Change the comparison to a >=, and always ignore
1 extra pixel. If we are so close to the bottom, the gradient is not
helpful any more.
Change-Id: Idf4c604142de8d2a803e1d94f3093cec8a8abb72
This is relevant in case the HTML looks like this:
<… class="reference"><a><span>[</span>2<span>]</span></a></…>
The additional <span> cause many additional mouseover and mouseout
events. The code already tries to filter these duplicate events that
are all triggered on the same link, but gets confused, especially when
the multiple chains of events overlap each other in unexpected ways.
It's a timing issue. This change does not fix the fundamental issue,
but does make it much less painfull.
This patch also removes the > from the selector. This is in case the
HTML looks like this:
<… class="reference"><span><a>[2]</a></span></…>
The additional inner <span> would prevent any reference preview from
being shown.
Bug: T214693
Change-Id: If2554ba78072245c27a1f85c46f33e3c58582c1d
Before, the element is still part of the DOM, shows up in the debugger,
and consumes rendering time (probably close to zero, but still). After,
the element is gone.
Change-Id: I580fd89868b5a91118a8115d71d819824ff7f80c
I actually tried to bring the animation for the "bottom" property back.
When finished, I realized this animation is done on *top* of the
horizontal scrollbar. This looks fancy, but is not what we want. We
need to keep animating the opacity.
This patch here contains a bit of refactoring that was left after I
went back animating the opacity.
Bug: T220200
Change-Id: Icf613f72f3baa3de86f8aa319667c8e8f16593fd
The changes in the patch check if a horizontal scrollbar is present
and move the absolute positioned fade overlay above that scrollbar.
Since we cannot change the CSS of the :after element via JS a new div
element was introduced.
Bug: T220200
Change-Id: Ia69c9be0facaf3ecebdb9f76d36f7cb3412c0816
The last line in all popups ends with a fade-out gradient to white.
The text in this last line can currently not be selected because it is
covered by this partially transparent container, and the container
consumes all mouse events.
This patch here moves the existing solution from "reference previews
only" (where this was a much more serious problem that on page
previews) to the top-level .less file that is for all popup types.
This is not strictly required, but I feel the code belongs there.
Bug: T220200
Change-Id: Iedf667ead453c9e72025d5fdc7af34756456e68a
This does not solve all two acceptance criteria mentioned in T220200:
* This will allow *interacting* with the scrollbar, no matter if it
is partly covered by the fade-out effect.
* This still does not place the scrollbar in front of the fade-out
effect. Very thin scrollbars are still very hard to see.
Bug: T220200
Change-Id: I394aea6a25c4b3923ad01e18328d42a5e50081f3
I had to disable ESLint to be allowed to upload this patch. It starts
complaining about something in code I did not even touched. The error
message does not make any sense to me (something about globals being
forbidden in code where I can not spot anything that would be remotely
global).
Change-Id: I6d4b178a65126c4b81b87d99142a6cdc845ae5ee
We run into this issue already one time, see Ifb5fe89 (T214710). The
exact same applies here.
The effect of this bug is that for certain references (typically ones
with a colon character in their name) the "jump down" link will be
broken.
Change-Id: Ic6723bd910cb5e5e1e1872ce39f2e271012245de
During story time on 2019-03-12 it was decided to consistently talk
about "References" in all messages. Main motivation is that this is
the term the community is most familiar with, and it is also the term
that is used in reference section headings most of the time.
Bug: T215063
Change-Id: Iaab8d2c0da1546a3c9d27bc8e2e1c784050ed135
For popup types other than references this particular margin is calculated
as "2 lines + 7", which results in 47px.
The calculation implemented with this patch results in 55px. That's 8 more.
This also increases the total height of reference popups by the same amount.
Bug: T214169
Change-Id: Ie7870717d2fd2ce78268d1fc1b79d87eff059318
See the discussion at I6bd7acc. In this codebase it was decided to not
use "classic" JavaScript, but document it with the benefits TypeScript
comes with. Since I27b5cb0 a dev dependency is in place that makes the
upper case JQuery type work.
All the lower-case jQuery this patch happens to touch have been placed
by me, not knowing better at this point.
Change-Id: I76ef8eabaf4850f07b140dac6f489df37422263e
We run into some confusion with the previous patch Ifcc355f, and
decided to talk about the max-height for reference previews later. The
result of this conversation is this patch here.
The mocks and discussions in T214169 mention several numbers that all
conflict a little bit. In detail:
* We ignore the mentioned "4 lines", but go with 5. We assume the 4 is
a mistake. Using 5 is consistent with all mocks and all given pixel
measurements.
* The mock ask for a max-height of 106px for the container. With this
patch it's 100px, which is exactly 5 lines. The extra 6px don't do
anything as far as we can tell, but make the code confusing.
* The mock asks for 215px for the whole popup. With this patch it's
203px. Note this number is (intentionally) not hard-coded anywhere but a
result from all line heights, line counts, paddings and margins combined.
Bug: T214169
Change-Id: I18f61ed02ed506d48e3834e2ebc48b3392f7d732
These icons are currently unused because the gateway does not deliver
the necessary information. This will be used starting with I6223cbb.
This patch aims to introduce all resources needed by the later.
Bug: T214908
Change-Id: Ie0c3c059222700169f2605c3123554c74d974256
This patch mainly adjusts the min-height and paddings for the reference
popups according to the mocks. It also enables scrolling inside of these
popups if the content exceeds the max sizes.
For now the scrollbars don't get specific styling. Also not, that the
fade out effect seen in the mocks is not part of this task.
Bug: T214169
Change-Id: Ifcc355fbcb6410778e7d4c569eb4cab09ed5dbf5
To make sure that we enable the link highlighting in the Cite extension we want
to trigger the click handler on the original footnote link. This is done by
passing the id of the source element to the model and the renderer.
Bug: T213905
Change-Id: I0bd59ac326269f3c0850946851fb79b611dc2a57
This CSS class is used by the regular wikitext content area, and used
by certain CSS selectors (e.g. the external web link arrow).
The DOM structure for each popup is (intentionally) created outside
of this scope, at the end of the current page's body. This works
great for page previews because the do *not* want to share any styles
with the rest of the page. But reference previews want to do this.
In this patch I also remove the inner <span>. It was misplaced (note
the name) and resulted in block elements nested in an inline element.
Bug: T214463
Change-Id: I740e37a2ed929edf971b348fbf20e5fb12012d37
This gets rid of a little bit of code duplication, and makes the
interfaces all conform to one standard again after I05ed4b8 left them
in a little inconsistent (but properly documented) state.
Bug: T214970
Change-Id: If8407c1a48aff1cb31fc2e74b3c2b846e79a3cb5
As discussed in Iaadcce9. This does have a few benefits:
* Less code in the already pretty big render.js file.
* The code setting the target attribute is much closer to where it
belongs: in the file that specifies how the content of a reference
popup should look and behave.
* The class name "mwe-popups-extract" is not mentioned in two different
files, but in the same.
Note this changes the signature of this src/ui/templates/… file to not
return an HTML string any more, but a jQuery object. The other templates
still return strings. I believe this is fine, and not that much of a
difference anyway. The signatures don't need to be identical. And the
jQuery object still represents the exact same HTML as before.
If it helps we could change all templates/… signatures accordingly.
Could be done in this or a separate patch.
Bug: T213908
Bug: T214970
Change-Id: I05ed4b886f79c5ae748f53ab9fed965dfd217620
I tried hard to keep the CSS as small and robust as possible. The
icon will be align with the text by adding a negativ margin. With
that we also decided against using RTL and LTR specific icons that
are positioned at the edge of the canvas for now.
Bug: T213907
Change-Id: I98888114e1c50e249cf31e71749323bd4f69da3f
I guess both is fine: either having the default in the gateway (as it
was before), or in the renderer (as this patch proposes). I, personally,
feel better with having it closer to where it is needed. This way it's
not possible to accidentially deliver a model object with an empty title.
The renderer will catch this.
At the moment we don't know exactly how we will fetch other titles (e.g.
"Book").
This change is split from I15611a4 where it was a little misplaced.
It also includes a test for the default fallback title.
Bug: T213907
Change-Id: I8ec3ddc21a417da7f95feff7b080cbd60d5472e7
Including tests. I also changed the title to include quotes as well,
even if not critical in that case.
Bug: T214754
Change-Id: I2f92a5714f7adc229a003f9167bcc9afdbc55583
Excluding tests for the renderer which keeps failing. This will be
readded in a later patch.
Bug: T213415
Bug: T213908
Change-Id: If79fa3d0a7a20f121b1ceda6e0e33ad691b1ad30
This adds support for preview popups on reference/footnotes from
the Cite extension. For that a new preview type was introduced and
integrated into the existing structures.
The essential starting points were this code comes into action are
added behind the feature flag introduced in the previous patches.
Bug: T213415
Change-Id: Ie0ccb03117bd654373d0f458b62cc52018361c67
