Commit graph

125 commits

Author SHA1 Message Date
jdlrobson 4b4e9d6399 Minerva should use the MobileFrontend eventBusSingleton
The talk overlay is created inside MobileFrontend, but the
overlay for creating a new talk overlay is here.

The two need to speak to either other - in particularly, the create
talk overlay must invalidate the current talk page before returning
the user to the former.

In preparation for the refactoring changes in MobileFrontend, the
same object is shared and the cache invalidation is moved here since
Minerva creates and manages the PageGateway instance that is given
to the talk overlay.

Additional change:
* Update a selector broken by changes in
I8c34646b7ba13a26facbb69684e65109870d27a1

Bug: T217102
Change-Id: I212ff044c4c608c6ea60a5fda043166cd434ec1f
2019-03-06 12:33:36 -08:00
Jan Drewniak 99aa7a28a4 HYGIENE: Remove element nav selector from mainmenu.less
The top level `nav` CSS selector (combined with the nested element
selectors, i.e. `nav ul li a`) represent a general DOM structure
which is not limited to the navigation menu and can interfere with
other styles.

This replaces the `nav` selectors (which have only been used to select
the main navigation) with a `#mw-mf-page-left` selector instead, since
that represents the main navigation as well.

Change-Id: I047108974fd295f196d9f7150c3721c05ac40c6d
2019-02-27 15:01:08 -08:00
jdlrobson 7fd843cd89 Fix talk overlay workflow
The talk overlay must subscribe to the creation of new topics
so that the list of topics in the talk overlay contains the
newly created topic. It does this by subscribing to the
talk-discussion-added event and forcing a route refresh when that
has completed.

Additional changes to browser tests:
1) QA: CSS selector changed for talk overlay
Since I42fd7b08c4b9d92dee549d06de8a0012ea037d28 the '.add' class
was removed from the talk button. This makes the browser test fail
but is a false positive.

2) One of the browser tests was using the same selector to mean
two different elements - the add discussion button in the talk overlay
is now clearly distinguish from the "add discussion" button that is blue
and appears at the bottom of talk pages

Change-Id: I935b3c5f37baf242c06585ae0e2f13d059b9c324
2019-02-25 14:14:53 -08:00
Stephen Niedzielski 2380e78f88 Hygiene: fix pageIssues QUnit test name
"cleanuptemplates" was the old page issues module name. The new name is
"pageIssues". Update the test module name.

Change-Id: Ie31e4d1548918463f6e33429ded3abc4bfb08dff
2019-02-21 11:41:26 -07:00
Stephen Niedzielski 7d4ca12a2e Hygiene: remove unused page issues function
Remove getAllIssuesSections(). This is no longer in use and does not
appear to be sufficiently general purpose to want to maintain.

Bug: T212371
Change-Id: I7ed73408705cba64b26dd318e78ae415b707e687
2019-02-13 01:34:32 +00:00
Stephen Niedzielski f54b4e75db Hygiene: move variable from file to local scope
Move allIssues from file scope to local scope.

Bug: T212371
Change-Id: I74693925ff0b20a36ec6acd53490cfde7273c984
2019-02-13 01:33:04 +00:00
Stephen Niedzielski 3dc9cff2c2 Hygiene: separate page issue view logic
- Move page issue view components that do not modify the DOM during
  during construction to PageIssueLearnMoreLink.js and PageIssueLink.js.
  PascalCase is used optimistically for filenaming in the hopes that
  these functions can become something like a JSX component. A "new"
  function prefix is used in the meantime.

- Move page issue view logic that munges the existing DOM to
  pageIssueFormatter.js. Substitute "create" prefixes for insert so that
  clients won't forget that calling the function is a modify operation.
  Alternative naming welcome but it shouldn't be confused with more
  idealistic components that do not depend on DOM state for
  construction.

- Consolidate createPageIssueBanner() and
  createPageIssueBannerMultiple() into insertPageIssueBanner() as the
  code was quite similar and were it a true component, it would probably
  be a single component.

All new files appear under page/ to keep their distinction from the
overlay code clear.

Some view logic remains in pageIssues.js but it shall be difficult to
isolate.

Bug: T212376
Change-Id: Iccce709c34fa8de5a28a5a00098add5775e3dc9a
2019-02-13 00:42:56 +00:00
jenkins-bot 42238a6be5 Merge "Hygiene: replace deepEqual tests with propEqual" 2019-02-08 00:02:55 +00:00
Stephen Niedzielski 118f4df828 Hygiene: replace deepEqual tests with propEqual
Replace QUnit deepEqual() assertions with propEqual(). The former is a
recursive == check, the latter is a recursive === check which seems
preferable.

  find tests -name \*.js|
  xargs -rd\\n sed -ri 's%deepEqual%propEqual%g'

Change-Id: I977244d24c47072cc62b7d9fc797505a5f39aa54
2019-02-07 14:55:36 -07:00
Stephen Niedzielski 5593b23aa8 Hygiene: replace mobile.startup/paths with props
Replace all occurrences of `M.require( 'mobile.startup/pathToModule' )`
with `M.require( 'mobile.startup' ).pathToModule`. Where multiple
requires existed, add an intermediate variable,
`var mobile = M.require( 'mobile.startup' )`, and dot off that.

This changes improves the consistency of MinervaNeue which currently
contains a mix of require styles and eliminates any deprecated requires.

Bug: T208915
Change-Id: If14f280672d914d07275197100b12421bb217b67
2019-02-07 14:55:04 -07:00
jenkins-bot 74a37b0412 Merge "Hygiene: revise lazyImageLoader.loadImages() API" 2019-01-25 20:52:49 +00:00
Stephen Niedzielski 0ce4fa8a66 Hygiene: revise lazyImageLoader.loadImages() API
There is no longer a need to pass in a jQuery dependency.

Don't use the stateful skin to query image placeholders. Use the lazy
image loader instead.

Depends-On: I3d023b3d96bf278666abb956142e5cee12b68b1f
Bug: T214658
Change-Id: I2bf42366c0e27462c32162124d07761b91d66166
2019-01-25 10:50:09 -07:00
Ed Sanders 63ead8075d build: Update eslint-config-wikimedia to 0.10.0
* Add inline exceptions for global selectors
* Exception for $.animate in scrolling code
* Fix $ prefixing in search.js
* Fix repeated selectors in search.js & toc.js
* Disable no-parse-html-literal in tests

Change-Id: Id58fe11d1e09714501a378e4ca7ed9588f02f32d
2019-01-24 17:59:40 +00:00
Ed Sanders 6c53e320bf eslint: Remove exception for computed-property-spacing
Change-Id: Ief22145c02379bdb3d37c96c5e1ef50f0ab519cf
2019-01-24 17:58:36 +00:00
jdlrobson db96b68d81 QA: Cleanup page issues browser test
* The check for whether the page issues code has been loaded should
be run just before clicking the banner. It's in the wrong place.
* Now page issues is live across all wikis, no need for "in beta"
check
* Seledctor can be simplified now no need to worry about beta
Change-Id: Ie24a9d9fc1966ca5db2cd0a6a37c1aa6d719924a
2019-01-22 19:47:02 +00:00
Stephen Niedzielski d1a20753fa Hygiene: revise lazy image loading API
Update the API usage for lazily loaded images. This is still clumsy and
may be further revised in future patches.

Bug: T211724
Depends-On: Ic73f78825eaab561e8ed694aa6cc102ccb471f95
Change-Id: Ia708cda688e6bdb12074d85d98f7e98fdf7b0ca8
2019-01-17 20:45:23 +00:00
jdlrobson 6d243c1df0 QA: Page Issues browser test should wait till JS has finished loading
Clicking the page issue banner (which is ready from first paint to
be clicked) will not yield the overlay. The user must wait for the JS
to load. Likely the reason for Minerva browser test failures against
beta cluster.

Change-Id: I06c488ca64dd44ad24368a1d6b47bb2646ad4552
2019-01-16 11:43:02 -08:00
jdlrobson b247beef3c QA: Run page issues browser test in beta
The test is failing in stable. Since page issues is going to production
next week this patch can be reverted as soon as that has happened

Change-Id: Idd8de17883006e3cc5f5615781a54a4072a78087
2019-01-15 10:08:05 -08:00
jenkins-bot 9e9442ecfc Merge "Constraint page issues CSS selector" 2019-01-09 01:11:04 +00:00
jdlrobson cb2634a78b Constraint page issues CSS selector
The 2 selector approach is flawed as the .ambox element matches a
hidden element in the old treatment which is not clickable.

I suspect this change will fix the failing browser test on the beta
cluster. Integration tests will continue to test the new selector
in the mean time.

Change-Id: I44a873b2e89069c4a47a428c528592159520568c
2019-01-08 20:23:37 +00:00
Stephen Niedzielski 672df850cb Hygiene: revise A/B test terminology
Improve the comments and APIs provided by AB.js:

- Control becomes unsampled.
- A becomes control.
- B becomes treatment.

This code does not appear to be in use presently, so it's a great time
to change it.

Change-Id: I31d619f889ee45102a4aed774a6ec41f0d95ba7d
2019-01-08 15:08:50 +00:00
jenkins-bot 9258806a0f Merge "pageIssuesOverlay is a factory function that returns an Overlay" 2019-01-04 15:39:48 +00:00
jdlrobson d29eca2bc8 pageIssuesOverlay is a factory function that returns an Overlay
This experiments with making PageIssuesOverlay an Overlay with
various options.

The appending of children is a little messy and points at a need
to standardise this some way
(see https://phabricator.wikimedia.org/T209647)

TODO:
* Remove the iconString property on PageIssueSummary which is no longer
needed

Bug: T209647
Change-Id: Iadd798a820dca6bbb31edc9a8570b6db7aac237a
2019-01-03 08:31:39 -08:00
jdlrobson c73ff1f7a5 Composition: DownloadIcon
The DownloadIcon is reduced to a factory function that
returns an instance of Icon

Depends-On: I4d703eef68d51bbe0b03579c5cca0845e17b8c9d
Depends-On: I4a4129b2cac7c7c49559beef0b8780f3211edf9c
Bug: T205592
Change-Id: Ib87390d17bef6f50842f52cd84c9ce2b162aaff0
2019-01-02 13:16:09 -08:00
Nicholas Ray 5bfd949f9a Remove usage of global event emitter from Minerva
This patch removes the remaining usages of M.on/M.off/M.emit
(functionality derived from moduleLoader.js in MobileFrontend) in
Minerva and continues the work of
Id990b0e1a53221d5c1cb3e3012aed0e27d801fc2 (patch for MobileFrontend).

This patch and the patch for MobileFrontend should be merged together as
they both depend on eachother.

Depends-On: Id990b0e1a53221d5c1cb3e3012aed0e27d801fc2
Bug: T156186
Change-Id: I005d2fcdbf91c2f1ac98178dfa388aa8174e7530
2018-12-06 19:30:09 -07:00
jdlrobson c31da9a236 QA: Drop protected page test
* On commit it needs admin rights which it doesn't have
* On browser tests the toast flakes too much.

Bug: T208808
Change-Id: I1fa93c8f451f3f839030fa7a144b1cb285c4239d
2018-11-29 16:51:04 -08:00
jdlrobson f055f8d789 Drop toast tests from beta cluster
Given the fade in/out animation of the toasts and the instability of
the beta cluster and the round trips to Sauce labs, we're seeing lots
of false positives on our browser test reports

Running these per commit should ensure we see minimal regressions and
get some protection. We'll continue to test other scenarios which do not involve
the toast on beta cluster.

Provided we can rely on green browser tests, we will trust the browser
tests more and they will be more useful than they currently are.

Bug: T208808
Change-Id: Idc601ad462de36f2d6d52fe951194b429e6f824f
2018-11-29 16:40:02 -08:00
jdlrobson 0a41aa53ca QA: Browser tests should limit write operations
We use lots of write operations in Minerva browser tests. On the beta
cluster many of these are redundant, as the page already has the content
required or the page already exists

Limit where we do our creation... the less write operations we make the
more stable we can expect these browser tests to be.

Change-Id: If88b878e14bf4a0424fcf23213653cfc2cf8d87b
2018-11-29 07:34:41 -08:00
jdlrobson b71bb0cc8c QA: Tagging changes from beta cluster
* features/search_loggedin.feature
They run on integration and are not super-business-critical.
Given their flakiness and Cirrus's likelihood of being down,
don't run it on beta cluster
* tests/browser/features/language.feature
Tag is superfulous, all scenarios repeat it.
* tests/browser/features/toggling.feature
ocassionally fails on firefox. Limit to Chrome and integration
tests
* tests/browser/features/search.feature
Limit the tests that run against beta cluster to search for
partial text and clicking a search result, since these are
two things we want to check integration well with other extensions.
The rest being testing on the more stable @integration tests
should be more than enough.

Change-Id: Ia2e8d3726212fee30725fdb9167ea38aa41eacbf
2018-11-27 17:53:00 -08:00
jenkins-bot bce934c54d Merge "QA: Remove hacks introduced for T170890" 2018-11-23 19:08:05 +00:00
jdlrobson 6a7d350160 QA: Page issues browser test is ready for new treatment
The page issues browser test is now compatible with both the old
and new treatments.

For consistency, in integration tests it will be run always in
treatment B since that will be the new way to do this going forward.

The beta cluster will test issue treatment A while it continues to be
the default. So with this we enjoy the best of both worlds.

When we enable page issues, the beta cluster will inherit the config
from production and we can remove the treatment testing for treatment B.

A follow up patch, might add testing for a page issue on talk pages,
which will retain treatment A for completeness if this is deemed useful.

Bug: T206647
Change-Id: I586523e452a6809e310f65a2ed55c6771d1965b6
2018-11-20 23:38:04 +00:00
jdlrobson 2922cc6fd1 QA: Drop more toast notification tests from Firefox (logged in search)
Toast tests are often failing in Firefox build.
They are also tested and pass more consistently in Chrome

Stable browser tests means real bugs get caught more often
so let's disable this browser test against Firefox for the time
being.

Bug:  T208808
Change-Id: I05d77eb53657bb3ea0daaad7906a50db6aab66db
2018-11-16 20:41:17 +00:00
jdlrobson 0638296fb3 QA: Remove hacks introduced for T170890
These no longer seem to be achieving their original
intention and may even be causing the
flakiness we experience now.

Additional changes:
* Disable some more tests in Firefox job

Bug: T208808
Change-Id: I735ec0ff293cfd7aa60519c080a300bd40dc0abc
2018-11-15 14:59:08 -08:00
jdlrobson 5e9739b9ff QA: Drop some tests in Firefox
The toast tests are flaking quite a bit in Firefox but pass in Chrome.
Having them run in both Firefox and Chrome seems less important than
being able to have better confidence in non-flaking tests so I'm removing.

Bug: T208808
Change-Id: I306518a7f3eb375715f6b9d6d599bf4f711ab6aa
2018-11-15 20:53:17 +00:00
jdlrobson f5acba28f3 QA: Remove redundant test so beta cluster tests pass
Currently beta cluster tests are failing because of a single test.

The test for whether a "tagline" appears on special pages is outdated. We now
show taglines on all special pages (defaulting to empty)

Let's thus remove it.

Additional changes:
* Rephrase "wikidata description" as "tagline" - we use the tagline to display
things other than wikidata descriptions e.g. the tagline on Special:MobileOptions

Change-Id: Icb66563cb3a5e7043ca41f59c826bd4247d89d52
2018-11-14 22:39:06 +00:00
jdlrobson f2639e7a73 QA: Don't check when_visible twice
Instead check the element is in the DOM before testing its
visibility.

This might help T208808 but it's a stab in the dark.

Change-Id: If7ccf5f2f03073c247de7fa497b3a6e31b570918
2018-11-13 16:49:24 -08:00
Bartosz Dziewoński 7b5928d795 Merge remote-tracking branch 'gerrit/page-issues-cleanup'
Bug: T198765
Bug: T208514
Change-Id: I9467101c9b01cbd7682de859e43e04966760eea6
2018-11-08 16:00:39 +01:00
jenkins-bot b08643cbab Merge "Fix undefined NS issue with @coversDefaultClass" 2018-11-01 14:22:48 +00:00
Alangi Derick 679a45677e Explicitly use "MediaWiki\Minerva\SkinUserPageHelper;" on-top of file
To make things neat, use "MediaWiki\Minerva\SkinUserPageHelper;" at the top
of the file and then mock class directly (shorter and easily readable).

Change-Id: Ie2ee64e75c38fff77d41af20bdea01015ed39a87
2018-11-01 14:35:57 +01:00
Alangi Derick e2e5cbc3d7 Fix undefined NS issue with @coversDefaultClass
In skin MinervaNeue, correctly cover SkinUserPageHelper
namespace in SkilUserPageHelperTest.php

Change-Id: I8c8bd9986c2b3989f093bfb585d90647e8da60ef
2018-11-01 14:28:20 +01:00
jdlrobson 72cd31f221 Remove page issues instrumentation
Note, since the page issues code is not feature flagged,
the new treatment will only be accessible via query
string until T206179 is taken care of.

Bug: T206178
Change-Id: I5ab2f3396e642f7b973263e2bb3963e0e82721b3
2018-10-23 12:19:25 -07:00
jdlrobson cb2ad9374e extract moved to parser
The extractMessage function has a lot to do with parsing - so this
and its tests are moved into the pageIssuesParser.

Change-Id: I62d79fbba166eff2c3ca573ef94ff86a269a7f9a
2018-10-23 12:17:45 -07:00
jdlrobson 878989bd85 Merge PageIssue and IssueSummary type defs
We have two type defs - IssueSummary and PageIssue.
I'd like to consolidate these two types by making
IssueSummary a combination of the two

Change-Id: Ic831b463fa66b0cacdd0b9b79aff741e55c0ec24
2018-10-23 12:10:22 -07:00
Alangi Derick a2fd0b3c1f Add "Discussion" button to Main page on Mobile View
* Enable discussion button on main page in mobile view so users
  can view discussion topics related to the main page on mobile.
* Update test to make sure only 'talk' and 'switch-language' actions are
  enabled on the main page and 'edit' and 'watch' are disabled on the main
  page.
* Minor typo fix for doc type. Use "array" instead of "Array" and CSS
  alignment fix issue with Discussion button when displayed together with
  Language selector button.
* [Suggested by: @Jdlrobson] Use a generic "a" CSS selector to style the ".talk"
  and ".language-selector" classes on the main page. This also avoids fixing the
  CSS for future buttons if added (future proofing the code), so if any other
  button is added in the future, the same css rule will be applied to it at once.
  Very wise idea from @Jdlrobson, thanks!

Bug: T206406
Change-Id: Iedce84595adc357f3a707f8b94d23b2ffea3476c
2018-10-23 19:59:22 +01:00
jdlrobson 13aeb1e8f5 Merge remote-tracking branch 'origin/master' into HEAD
Change-Id: I2b15e07932e4fe8b2a49fa87bb6136a3b50b7786
2018-10-22 16:20:33 -07:00
jenkins-bot 53216161c6 Merge "Fix watch star browser test" 2018-10-19 18:27:53 +00:00
jdlrobson 1c034f1d73 Fix watch star browser test
In I06ef42cb1461fde7ca0aa903f174c3b1f39ab154 a button became an
a tag and somehow that got through the Jenkins censors.

With this change Jenkins on Minerva will be happy again

Bug: T207480
Change-Id: I3faedb10e46c3e3e237f5b90d71ffe20606fce6d
2018-10-19 11:00:19 -07:00
Stephen Niedzielski f07985c6de Hygiene: move page issue group parsing to parser
Separate the page issue grouping concern so that changes to parsing
don't concern everything else and vice-versa.

Bug: T203449, T202349
Change-Id: I7bddb0c53310805ece71b8f7821b1d6ce05cfae9
2018-10-16 18:20:38 +00:00
jdlrobson cbad0bd363 Merge skin option modules into a single ResourceLoader module and move enabled logic to client
It's presumed that skin options will eventually become the default or be removed from the skin.
While they are not the default, it would be helpful to package them in one single module - as ResourceLoader
modules are costly bloating the dependency graph in the MediaWiki startup module.

In T167713 we talked about grouping our entry points by page rather than feature, which this seems consistent
with. A page with special options enabled is different from a page without.

Change-Id: Id948f913d4743532ba3442d2059a03c122419ff2
2018-10-12 10:18:21 -07:00
Bartosz Dziewoński cada514ded Remove code related to no-anonymous-editing mode of MobileFrontend
All of this would only be used with the configuration setting
`$wgMFEditorOptions['anonymousEditing'] = false;`.

Removed features:
* Call-to-action popup in skins.minerva.editor (note that anonymous
  editors still get a CTA from MobileFrontend's EditorOverlay code)
* Pointer towards the edit button shown after registering via the CTA
  (entire skins.minerva.newusers module)

Bug: T205382
Change-Id: I66c7035f7a23581811dda87c911dea41d4a8e5da
2018-10-10 19:03:53 +00:00