Commit graph

102 commits

Author SHA1 Message Date
jenkins-bot 1ad76e7d13 Merge "Users can request new treatment via query string" 2018-09-20 19:02:59 +00:00
jdlrobson 2f3fed5f4a Users can request new treatment via query string
Bug: T204746
Change-Id: Icea989a401a5de8772fdcd9f25f100899bc1c6f3
2018-09-19 15:21:32 -07:00
jdlrobson 01dc9de92e Minerva is responsible for turning on WikimediaEvents itself
Unlike other skins, Minerva wants to be in charge of when WikimediaEvents
is loaded, so that it can guarantee load order happens at a time that
suits it.

WikimediaEvents will be loaded after initialisation of the interface.
This allows Minerva to enable schemas such as ReadingDepth conditionally.

Upon merging this patch, Ibb45f40ea301727c0c6480043760bd9426106845 can
be merged which will revoke WikimediaEvent's ownership.
Merging in this order will ensure that ReadingDepth  is never
removed from production.

Bug: T204144
Change-Id: If8395033f31485aca0ca3b38fda1be985369b481
2018-09-19 22:09:26 +00:00
jdlrobson 03bc2518d4 Red links are linked separately
In I1a471f81cc9390fc9e8665a7a336cf2dd7a063ab we excluded
edit events that led to the creation of a new page.

This restores them by providing a dedicated home for this data
in the schema.

Additional changes:
* Address some line length warnings

Bug: T204073
Change-Id: Ie7eb95f15737e94b7926d38ed6411bc0e5df2404
2018-09-18 13:43:56 -06:00
T. Bayer 287a2aafe9 Exclude redlink clicks from modalEditClicked event
This event is meant to track attempts to edit the page we are on,
but also happened to catch clicks on "see talk page" links, in
case that talk page doesn't exist yet.

Bug: T204073
Change-Id: I1a471f81cc9390fc9e8665a7a336cf2dd7a063ab
2018-09-18 13:37:34 -06:00
Stephen Niedzielski ba17b11987 Hygiene: fix line length violators
Change-Id: Ie1619acf5f319dafc2c9dc612335c1208522dab8
2018-09-13 09:33:20 -06:00
jenkins-bot 5145bc1818 Merge "Fix formatPageIssuesSeverity" 2018-09-07 14:48:27 +00:00
jenkins-bot 6f7e652f32 Merge "Hygiene: Consistent error naming" 2018-09-07 14:17:22 +00:00
jdlrobson f14d9514c6 Fix formatPageIssuesSeverity
The page-issues reducer function that retrieves the severityLevels
for issues was incorrectly comparing two values in the multiple-issues
scenario.

Instead of comparing the severity of the current issue with the previous
value in the accumulator, it was comparing the current issue with the
previous issue in the original array, thus ignoring the
previous `maxSeverity` value in the accumulator.

Tests added and function refactored with more explicit variable names.

Bug: T203725
Change-Id: Ia4c15cbf0c2457d68a7381e8ed04c1a6b3ae65d1
2018-09-07 11:25:16 +02:00
jdlrobson c68f7ba09f Issues instrumentation: issuesClicked events should always send 1 sectionNumber
Erroneously in I6fd55c35b9e2ce35894259f36d1a50fb5dca5e43 for the old treatment
 we sent sectionNumbers for all issues in the page.

This is inconsistent with the issuesSeverity field above it
Add a clarifying inline comment.

Bug: T203050
Change-Id: Ib1fcda0c49a162cd7aca8ee8b3221236f724e1d7
2018-09-06 17:40:34 -07:00
jdlrobson e7f0cfd665 Hygiene: Consistent error naming
Rather than using err and error as variable, use error for consistency
Follow up to I07f01b4c025b2e5e4cbf88ec05e7c536442c62cc

Bug: T202026
Change-Id: I54165ff1f1b17284d8232c491244e1a98950d5e2
2018-09-06 21:04:53 +00:00
jenkins-bot 993e77e76b Merge "Correct the link for page issues in the old treatment." 2018-09-05 20:14:06 +00:00
jenkins-bot 002160ae9c Merge "page-issues sectionNumbers consistency on modalClose event" 2018-09-05 20:02:35 +00:00
jdlrobson dc6f707ea1 Correct the link for page issues in the old treatment.
createBanner was incorrectly always assigning the 'all' keyword
to the old page issues banner. Instead it should use the section
number in the function signature - as this decision of which issues
to show is made inside initPageIssues

For the old treatment, in the main namespace, we only show issues
in the lead section. When we use the 'all' keyword the visual is
the same, but it breaks the instrumentation requirement that
sectionNumbers and issuesSeverity should be the correct length

Note, for the talk and category pages treatment, we do not
log any events so the instrumentation doesn't matter here
and the 'all' keyword correctly targets all issues in the page
as before.

Bug: T203050
Change-Id: I63e45da05ca033fe282633f7fd59038a8e5d8c8d
2018-09-05 12:51:56 -07:00
Jan Drewniak 1a9bcd1e8a page-issues sectionNumbers consistency on modalClose event
For the page-issues modalClose event, the number of values for `sectionNumbers`
and `issuesSeverity` should be the same, since `sectionNumbers` should describe
the the section of each visible issue in the modal, not the section of the
modal itself.

Bug: T203050
Change-Id: Ic58c5940a6059e71aa3aeed26232afbe8faf1618
2018-09-05 21:29:53 +02:00
Timo Tijhof c1fe42fede Hygiene: Use early returns in a few places for special cases
When handling special cases that are logically distinct from
the function's main branch, it improves code quality (through
readability and maintainability) to place those first and with
an early return.

The has the benefit of the main return statement being easy to
find at the end of the function. (Not early and/or in a block).

It also means when working on the code, there is generally a
less complexity and fewer nesting levels, given that most code
is in the main branch. This makes is easier and quicker to verify
that code does what it should, as well as making it easy to
extend in the future. When considering to add code to end of a
function's main scope, it should relate to the function's main
branch by default, not a special case. For example, a getName()
method should not end with a top-level statement 'return false'
(unless it is a stub). Rather, one would expect it to end with
`return name`.

Change-Id: I1f3088f2409c82dd3bf757fc8fa27dc97ae2767b
2018-09-04 21:08:00 +00:00
jenkins-bot 34ead3f8c5 Merge "Client side error reporting for Minerva+MobileFrontend" 2018-08-30 22:40:19 +00:00
jdlrobson cd528e120b Client side error reporting for Minerva+MobileFrontend
This adds a client side error logger that will store errors
in EventLogging. We will use it to get a sense of the number
of errors inside mobile.

To enable make use of the new configuration variable
wgMinervaErrorLogSamplingRate = 1

Notes:
* the optional `meta` field will not be utilised by the generic
error handler.
* URI length is not trimmed for title. We will consider whether we need
to make any adjustments to the schema values during roll out.
** For stack trace, we limit the length by removing errorUrl (which is logged
separately)

Testing:
Add throw new Error('asasasa'); anywhere in your code and make
sure the code executes. It should trigger an Error event provided
that configuration has been updated.

Bug: T202026
Change-Id: I07f01b4c025b2e5e4cbf88ec05e7c536442c62cc
2018-08-30 14:35:27 -07:00
Jan Drewniak 815f3d99ee For page-issues pageLoaded and editClicked events, treat
"multiple issues" templates as one issue.

When logging the `issuesSeverity` and `sectionNumbers` field,
any issues that are part of a "multiple issues" template only send
one value.

Adds an `isMultiple` property to IssueSummary to determine which
issues are part of a multiple-issues template.

Bug: T203050
Change-Id: I7d55dfead72439df4accadcdc8623a080e1321c2
2018-08-29 13:59:20 -07:00
Jan Drewniak 2cbd57c2f3 Consistent sectionNumbers and issuesSeverity for page-issues.
The value of `sectionNumbers` should be the section number of each issue
Not the number of the sections that have issues.

Bug: T203050
Change-Id: I6fd55c35b9e2ce35894259f36d1a50fb5dca5e43
2018-08-29 13:45:55 -07:00
jenkins-bot 198276256e Merge "Avoid abrupt and jarring white flash in media viewer" 2018-08-28 19:42:04 +00:00
jdlrobson 0acfda1931 Don't send NaN as a value for sectionNumbers
The keyword "all" was being parsed as an integer incorrectly. To avoid this

Bug: T202940
Change-Id: I5553a4bb50cd7639c879f2c6e812fba25a216175
2018-08-28 18:26:09 +00:00
Zoranzoki21 6d877928f5 Fix common typos in code
Bug: T201491
Change-Id: I1b31983069a45ff6c0493f370596351a407f6a87
2018-08-27 20:54:16 +00:00
Jan Drewniak ae19031f8e Log sectionNumbers in PageIssues schema
Adds logging for the sectionNumbers field in the PageIssues schema.

Additional changes:
* createBanner now requires section number to be a string - this ensures
consistency with how these are used.
* fix a bug which meant createBanner was being called with undefined
section number (due to table of contents)
* Fix some indents in some JSDoc blocks
* Change parameter in function signature from mixed type (int or string)
to explicit string
* update schema number

Depends-On: Ia2696b86c6855d7b46a3f668585377d106d7af23
Bug: T202098
Change-Id: I20511a77258ea245f3d6fe93ade238e5df397a71
2018-08-27 18:02:51 +00:00
jdlrobson ee36bcfccf Don't send issuesSeverity given that's an event default
An event only needs to send this field when overriding the
default. This line is thus unnecessary.

Change-Id: Ia1dfcac5dd25f36f5f4169daf6535fd66aedf754
2018-08-27 10:52:55 -07:00
jdlrobson ba064c2013 Avoid abrupt and jarring white flash in media viewer
Previously, the image overlay would always be loaded asynchronously.
Given clicking left/right opens a new mediaviewer this left a jarring
white flash. By loading the image overlay synchronously after the first
has been loaded, we avoid this flash.

Note, the task does propose preloading and animating the images
but this is a much bigger change and deemed out of scope for this
particular task.

Additional change:
* Use a shared mw.Api instance for ImageOverlay and PageGateway

Bug: T197110
Change-Id: I28d06b34cdea4fedcd7fb754572191e904ecc81a
2018-08-23 22:55:15 -06:00
jenkins-bot cd6ef504cf Merge "Make Deferreds promise compatible" 2018-08-22 22:33:52 +00:00
jdlrobson 56a10c40e0 Make Deferreds promise compatible
This patch updates the various usages of $.Deferred for loading
overlays in routes to be ES6 Promise compatible

Bug: T188937
Change-Id: I3fc24bf3471a99e7671d1191bdd46cb741286ee1
2018-08-22 14:27:48 -07:00
Stephen Niedzielski 73131b7b45 Hygiene: move page issue overlay to distinct file
Move page issue overlay specific code out of cleanuptemplates and into
PageIssueOverlay to clarify what code is specific to the page issues
modal screen and what's specific to the page itself.

Bug: T191528
Change-Id: I95821ccda84306ddd5d22b57ffbae8d13ca44408
2018-08-22 13:42:42 -06:00
jdlrobson 65f2e5ef51 getPageToken now defined in core
Bug: T201124
Depends-On: Id43e9d3f3e687133c98017d24c10c307a5a855a1
Change-Id: I2b519361dfa974acef918e00ffc1ae14e33e28f5
2018-08-21 20:33:35 +00:00
jdlrobson 7962ba87a5 Docs: Present classes rather than modules
Use classes rather than modules for the various Views inside
Minerva.

Change-Id: I849ae50e48c01012ba6713ec3bf5f1c836ef22bc
2018-08-20 16:43:41 -07:00
jdlrobson eea19f3e18 PageIssues should use new standard pageToken getter
Rather than inventing its own page token the PageIssues
schema will rely on the standard page token.

In future when I9bb18d47e36d2d99d812e5b47ea9516d3dff3a16 is
merged this will mean that PageIssues and ReadingDepth have
consistent page tokens.

Bug: T201124
Change-Id: I19fea41e04cd792423ea9d41a673563418b69726
2018-08-17 10:28:17 -07:00
jdlrobson 40eca4e3f3 Hygiene: IssuesOverlay moved from MobileFrontend to Minerva
The CleanupOverlay is moved to Minerva and renamed the
IssuesOverlay to be consistent with current terminology

The new IssuesOverlay is defined inside the module
skins.minerva.scripts to which it now belongs.

Additional changes:
* various file renames
* overlay-cleanup renames overlay-issues
* cleanuptemplates renamed issues.js
* Add a test stub file to avoid the need to load templates inside
the test environment

After this change, I75f47622d94e504688e04dfb2892540473817053
should be merged to avoid confusion.

Change-Id: I08945a324a6b878abe56efed1e988466085b3018
2018-08-16 14:06:12 -07:00
jdlrobson 6a5b2e284d Allow subsection issues
Depends-On: Iaa35317cdd96a1ad306ab38dc3ab2f18cc7bb31f
Bug: T197932
Change-Id: I84649dc80192729934f7bbb23a8e78296717bf36
2018-08-16 11:51:10 -07:00
jdlrobson 6718bc4351 Drop issues on editor
T196150 disabled the fallback editor when JavaScript is available.
As a result the code for page issues on the editor screen is redundant.
This simplifies our code.

The css rules relating to editor
issues are retained, given they simplify the editor screen for mobile
devices.

Bug: T196150
Change-Id: I76d586414cdd8de533bb891b87d1e0b7efc20a68
2018-08-16 11:43:28 -07:00
Stephen Niedzielski ac245a7d97 Hygiene: don't hardcode page issue header selector
When parsing page issues by header, use the MobileFrontend selector
symbol, Page.HEADING_SELECTOR, instead of hardcoding it. This selector
is configurable by the extension.

Bug: T191528
Change-Id: I7c9db38f9102bfef0a98784be32c7e4707d747e5
2018-08-15 19:40:49 +00:00
Stephen Niedzielski 22b2f0fd7c Hygiene: move page issues A/B test logging to file
Refactor the page issues A/B test logging implementation to a distinct
new file that only has the responsibility of tracking.

T191528 is referenced in this commit as I was having difficulty
answering the feedback and bugs reported in the current implementation
without working through and restructuring the flow as I understood it.
This refactor is merely a byproduct artifact of that effort to focus on
the parsing and presentation responsibilities.

Bug: T191528
Change-Id: If547a0a67fbc9a532f834fe374abf668309e73df
2018-08-14 20:49:06 +00:00
Stephen Niedzielski 061d155303 Hygiene: remove unused function AB.onABStart()
Callback usage was removed in I67fb6e448f6ecc97c89c1187e491ee05f7a312ef
so this function may be removed.

Bug: T191532
Change-Id: I88f0d6740c9e9615faba2e3c60772269c705f43e
2018-08-09 20:50:51 +00:00
Jan Drewniak f67c410859 Page Issues AB test event-logging instrumentation
Uses `mw.trackSubscribe` to create an intermediary data handler
named `wikimedia.PageIssuesAB` which extends event-logging data
before passing it to the eventLogging through `wikimedia.event.PageIssues`.

Event hooks are placed where appropriate and the `CleanupOverlay`
class is extended to capture events from within the page issues
modal.

Additional changes:
* Merge two identical on click event handlers for
.edit-page, .edit-link elements
* change pageIssueParser.maxSeverity to accept an array of severity levels
instead of an array of pageIssues objects

Depends-On: Ic84e4a3286220407863167e0f57cef1b13a72964
Bug: T191532
Change-Id: I67fb6e448f6ecc97c89c1187e491ee05f7a312ef
2018-08-09 20:30:12 +00:00
Stephen Niedzielski 765a46c464 Fix: add a few more POV templates to issue parser
Bug: T191528
Change-Id: I777921798fee447dc3c954a585e418371e2866e8
2018-08-09 09:01:45 -05:00
Stephen Niedzielski a6ba0f2c77 Fix: page issue icon type determination
The page issuer parser works with plain old Elements, not jQuery. Pass
in the expected type.

This addresses problems with the icon display for ambox templates relating
to balance (see https://phabricator.wikimedia.org/T191528#4465069)

Bug: T191528
Change-Id: Ie4a34087f9f1700342cc54a50cd3272fc9ea34ee
2018-08-08 17:09:03 -07:00
jdlrobson 100b7951a2 Restore page issues
Follow up to I96808541d48be7869fed3bc30babb80866e139ec
which incorrectly wired up the page issues feature.

Bug: T200867
Change-Id: I7c3198f95dae1f7c5e0991dca651dd64949b3615
2018-08-01 11:34:20 +08:00
jdlrobson 9644e5372b Add tests for page issues code
Bug: T191532
Change-Id: I5291b51ec32ab95ff1ec41e50b84fea4567f28cb
2018-07-31 11:42:09 +00:00
jdlrobson 0ca42ee64e Hygiene: Separate cleanups library from initialisation
In order to write tests, we'd like to separate code without side
effects from code that executes it as part of setup.

This shuffles dependencies and makes page and overlayManager
parameters to the init function (injected dependencies)

Change-Id: I96808541d48be7869fed3bc30babb80866e139ec
2018-07-31 11:42:04 +00:00
jdlrobson 3d7007f6a5 Hygiene: Remove anonymous function and indent
The readability of cleanuptemplates is hurt by having an anonymous
function embedded inside.

This patch wraps initPageIssues with the jquery ready call and removes
the function wrapper and decreases the indent of its contents.

Purely stylistic change.

Change-Id: I44b2371add32780f96160e0075787a9ea7222f39
2018-07-31 11:41:59 +00:00
Jan Drewniak e27cbf2a49 Reading depth hook should send additional sampling bucket parameter
The hook that enables the Reading depth test should send an
additional paramter that specifies which test bucket the hook
being is calling from.

Bug: T191532
Change-Id: Ifd9f43220c476ece8a0c0cee46b62b58a717c616
2018-07-31 11:40:29 +00:00
Jan Drewniak a10bb49c01 Truncate page-issues to 2 lines of text.
Truncates ambox template styles to only show 2 lines of text.
"Learn more" link is placed on the same line as message text
and a fade is applied so that it doesn't overlap with the message text.

On table & larger layouts, the "learn more" link is placed on its
own line below the page-issue text.

Bug: T197931
Change-Id: Id607dea537c212298c02a0e1639aef2a786eb424
2018-07-26 08:14:08 -05:00
jdlrobson a3efca1582 Remove unnecessary skins.minerva.base.styles
* truncated-text is only used in MainMenu, last modified bar,
 a few special pages
Making this a mixin and limiting it to mobile.startup skinStyles
and mobile.special.styles thus makes sense
* cloaked-element is only ever added by Skin.js in mobile.startup
* position-fixed is only ever added by Skin.js

Change-Id: Iaf7061e9dda87eb25c963f8a281175af08f358e5
2018-07-26 01:18:00 +00:00
jenkins-bot ef63ced9b4 Merge "Match section issues to section number" 2018-07-24 23:03:18 +00:00
jdlrobson fcaf244cc4 Hygiene: lazy loading animations do not need to be render blocking
The transition only happens when images are loaded via JavaScript
so limit the styles to a file loaded via JS NOT render blocking
CSS

Change-Id: I56661db13e7fbb400a05b13c369d8598df449297
2018-07-23 13:57:42 -05:00