Commit Graph

68 Commits

Author SHA1 Message Date
Jdlrobson 8d703b7758 Remove all main_menu tracking related code.""
This reverts commit de353c06c9.

Bug: T228681
Change-Id: Idfd964c31e09e271dca9ac39f7932df49cdcb5f6
2019-08-01 10:40:09 -07:00
Stephen Niedzielski 93f930ce3e [UI] [menu] remove menu height resizing
Remove the page action overflow and user menu height sizing. Previously,
a maximum menu height was set so that the menu itself would scroll. A
minimum height was also used so that the menu couldn't be shrunk down to
a silly size. Both the minimum height LESS and maximum height JS are now
removed.

Bug: T225959
Change-Id: I201374ab8b249272ee5dbb1401b844ffe034ea66
2019-07-31 20:05:54 +00:00
Roan Kattouw ec17da76c1 Improve tab scrolling logic
* If the leftmost tab is selected, scroll all the way to the left
* If the rightmost tab is selected, scroll all the way to the right
* If a tab in the middle is selected, scroll to center it
* If the selected tab is wider than the tab container, make sure its
  start (left edge in LTR, right edge in RTL) is always made visible.

As Bartosz reminded me, .scrollLeft in RTL is a cross-browser nightmare
(see https://github.com/othree/jquery.rtl-scroll-type), so add a bunch
of code working around this. Some of this logic is in OOUI already, but
what's there is not enough for what we need here, and we also don't want
to load OOUI for this.

Bug: T223142
Change-Id: Ica298954b42f9daa4819043ec24bc0266290a927
2019-07-30 18:35:14 -07:00
jdlrobson 0443f9936f Dev: Simplify red link drawer
* Remove event and add class .hide so that it uses event defined in
Drawer class
* Use actionAnchor - it achieves the same purpose

Change-Id: I2570de1e692a22c9383a850460eabb75b3d8eaf5
2019-07-30 08:29:29 -07:00
jdlrobson afecb58a88 Restore desktop MMV
Changes in the MultimediaViewer extension mean Minerva's onClick event
is preventing the default of the click handler in MultimediaViewer.

Explicitly do not setup our multimedia viewer version when the desktop
MMV is present!

Bug: T227034
Change-Id: I94e22d045346d53bb2c50035281398799fb6c7ac
2019-07-29 23:36:16 +00:00
Stephen Niedzielski 0a4f5b6126 [UI] [new] add user menu
Add new user menu. The changes required include:

- Break up AuthMenuEntry into reusable components. They're now simple,
  independent, static functions in AuthUtil that are easy to reason
  about and compose.

  There's lots of verbose code because of the builder and director
  patterns. That is, most of the code is for building the thing we
  actually want to build instead of just building it. It's easy to write
  but no fun to read--even simple configurations are extremely verbose
  expressions that must be threaded through the system.

  These builders are also single purpose and unlikely to be reusable
  unlike a URI builder, for example. As objects, they're not especially
  composable either.

- Similarly, break up Menu/DefaultBuilder into BuilderUtil and ban
  inheritance. Inheritance has not worked well on the frontend of
  MobileFrontend. I don't think it's going to work well here. E.g., I
  could have made changes to the base class' getPersonalTools() method
  such that the client passes a parameter for the advanced config or
  maybe I just override it in the subclass. In either case, I think it
  makes the whole hierarchy nuanced and harder to reason about for
  something that should be simple.

- Add ProfileMenuEntry and LogOutMenuEntry for the user menu.

- Rename insertLogInOutMenuItem() to insertAuthMenuItem() which matches
  the entry name, AuthMenuEntry.

- Extension:SandboxLink is needed to display the sandbox link in the
  user menu.

- Performance note: the toolbar is now processed in MinervaTemplate,
  which corresponds to removing the buildPersonalUrls() override.

- To mimic the design of main menu, the following steps would be
  necessary:

  1. Create a user/Default and user/Advanced user menu builder and also
     a user/IBuilder interface.
  2. Create a user/Director.
  3. Create a service entry for Minerva.Menu.UserDirector in
     ServiceWiring. The Director is actually powerless and doesn't get
     to make any decisions--the appropriate builder is passed in from
     ServiceWiring which checks the mode.
  4. Access the service in SkinMinerva to set a userMenuHTML data member
     on the Minerva QuickTemplate.
  5. In MinervaTemplate, access the userMenuHTML QuickTemplate member
     and do the usual song and dance of inflating a Mustache template.

  This patch does everything except add a service, which was agreed to
  be unnecessary, so that logic is now in SkinMinerva.

- Wrap the existing echo user notifications button and new user menu
  button in a nav element. This seems like a semantic improvement.

- The existing styling and logic for the search bar and search overlay
  are pretty messy and delicate. Changes made to that LESS endeavored to
  be surgical. There's lots of room for improvement in the toolbar but
  it's out of scope.

- Rename logout icon to logOut.

Bug: T214540
Change-Id: Ib517864fcf4e4d611e05525a6358ee6662fe4e05
2019-07-24 18:24:07 +00:00
Pmiazga de353c06c9 Revert "Remove all main_menu tracking related code."
This reverts commit d11c84d08b.

We decided to track both old MobileWebMainMenuClickTracking and new MobileWebUIActionsTracking for some time. Then once everything goes stable and it's proven to work correctly we will merge d11c84d08b.

Bug: T220016
Change-Id: Ib4d52e8b8c870774041284e575564a9933af6136
2019-07-22 18:31:55 +00:00
jenkins-bot 9352a70023 Merge "Remove all main_menu tracking related code." 2019-07-19 16:24:49 +00:00
Piotr Miazga d11c84d08b Remove all main_menu tracking related code.
This code should live in WikimediaEvents extension, not in
Minerva.

Bug: T220016
Depends-On: Ic2d6d1b21b0eb72ad68b0c447bc63f7d1bb021f4
Change-Id: Iaeb12704dcd257f0783f1ebec3def01cb2848228
2019-07-19 15:30:28 +00:00
jdlrobson 0a4f88b317 router.navigate is deprecated use router.navigateTo
The navigate method does not accept a second parameter for
replacing state. navigateTo was what was wanted.

Mark navigate as deprecated (to match ooui router) to make
sure this doesn't happen again.

Bug: T226106
Change-Id: Ie86fa632b2175727417b166c222c29bdf12bfea4
2019-07-16 16:50:58 -07:00
jdlrobson a9d73060fa Restore table of contents and error logging
When moving to packageFiles, the modules errorLogging and toc
were not imported by init.js meaning they were not run.

ResourceLoader should probably error in this situation, but until
then we should fix this problem.

This is a follow up to I44790dd3fc6fe42bb502d79c39c4081c223bf2b1

Bug: T212944
Change-Id: I86efb7be1c39b03f63c8f1e0b107216cd30ff6de
2019-07-16 14:45:35 -07:00
jdlrobson 72df451bd3 Embrace packageFiles
Help with readability by using module.exports and require rather than the MobileFrontend
provided mw.mobileFrontend module manager (and avoid adopting webpack at this time)

Replace usages of mw.mobileFrontend.require with local require and module.exports
(compatible with RL or Node implementation)

Changes:
* Notifications modules are merged into skins.minerva.scripts and initialised
via a client side check.
* new file overlayManager for exporting an overlayManager singleton
rather than being hidden inside resources/skins.minerva.scripts/init.js
* All M.define/M.requires swapped out for require where possible
The `define` method is now forbidden in the repo.

Bug: T212944
Change-Id: I44790dd3fc6fe42bb502d79c39c4081c223bf2b1
2019-07-16 18:04:10 +00:00
Nicholas Ray 331df226f5 Make Minerva use new PageHTMLParser.js and refactored Page.js
In I02f8645aac1d7b081eaba8f2ac92a2ef51f78182, Page.js was made into a
model and its html parsing responsibilities were moved to
PageHTMLParser. Additionally, a singleton for the current page
(currentPage.js) and a singleton for the curent page html parser
(currentPageHTMLParser.js) were introduced to replace the usage of
M.getCurrentPage().

This commit refactors Minerva to make use of these changes.

Notable changes:

* 🐛 The event bus singleton was added to references.js since it
previously relied on an instance of Skin to listen for the
`references-loaded` event. However, this event is emitted on the event
bus singleton.

* Additionally, I didn't see a reason why the `references-loaded` event
needed to also pass the current page instance when the only file
listening to it is references.js (which already has the current page
instance) so I removed that and the convoluted passing of page.js within
the file. I assumed this logic was unecessary.

* The call to drawer.showReferences in references.js now was made to
pass the currentPage instance as well as the currentPageHTMLParser. This
is to prepare for I6e858bbe73f83166476b5b2c0fdac1cca7404246 where the
getReferences() interface for ReferencesMobileViewGateway.js and
ReferencesHtmlScraperGateway.js is refactored to accept both of these
instances. Additionally, references.js was refactored to support event
delegation which gets rid of some parsing/setup logic.

Bug: T193077
Depends-On: I02f8645aac1d7b081eaba8f2ac92a2ef51f78182
Change-Id: I2f32dbcc4ebaa4bebb297cda1ecce3457922b343
2019-07-15 16:43:51 -06:00
Roan Kattouw b64b4c1b9f Don't scroll vertically when scrolling active tab into view
Do the scrolling math manually, and only do horizontal scrolling on the
tab container. Don't do vertical scrolling, and don't scroll the
viewport.

Bug: T226112
Bug: T223142
Change-Id: Ie67e15bd51252906897a213505ae82cf29ba5cf2
2019-06-24 15:18:56 -07:00
Stephane Bisson 4f5dbf7d55 Make the tabs scrollable horizontally
* Add a container around the tabs and use flexbox
  to position the tabs on one line and make it
  scrollable.
* Add some JS to scroll the currently selected
  tab into view.

Bug: T223142
Change-Id: Ie2205e6836797c2ac000e12a01f78a4aa7bc5b81
2019-06-14 13:50:23 -04:00
Ed Sanders 12e6a2b9d5 Add missing dollar prefix to jQuery variable
Change-Id: Ib6ea281b749e040d43c2dc3558a0a96ce59b7385
2019-06-12 21:29:49 +01:00
Ed Sanders 8e352f7824 Connect initMediaViewer to wikipage.content hook
This means if new content is added to the page, it
will be connected to the media viewer code.

Depends-On: I79caa6d1b83ec2a618ca703607165197add4e100
Change-Id: I9936ee1decaa5de6677bc33ebe726ff6253d6ac5
2019-05-08 18:41:22 +00:00
Stephen Niedzielski 815f3386e3 Update: add secondary page actions submenu in AMC mode
When advanced mobile contributions mode is enabled and
`$wgMinervaOverflowInPageActions` is set, show a secondary overflow menu
on main namespace articles and user namespace pages. The menu content
varies for each namespace. The new submenu is *disabled* by default,
even when AMC is active. This feature should not be deployed until
instrumentation is available.

The secondary menu is rendered in PHP using the existing menu system
with some changes to the template. The checkbox hack is needed for no-
JavaScript keyboard navigation until :focus-within is supported. CSS
additions are documented in the source.

All client side toolbar execution occurs in Toolbar.js. Enhancements are
documented in the source.

Minor changes to the existing download button:
- Move download and edit button initialization to Toolbar.
- Update copy (not visible) from "Download" to "Download PDF".
- When the overflow menu is present, use the "hasText" / label style.

Wikimedia UI icons are copied from OOUI at d00a0ac and seem useful to
expose as HTTP URIs (not data URIs).

The overflow menu does not show for pages provided by the content proxy
since its entries depend on $tpl->data['nav_urls'] being populated.

Bug: T216418
Depends-On: I0781151a8232b6a7b52f79a34298afcecb8e4271
Change-Id: I4b50a0e519024a7ab91dae6ab40b23cf14a03861
2019-04-19 14:51:17 +00:00
Jan Drewniak 7821b1499c Removing code related to old page-action menu markup
Bug: T218653
Change-Id: Ifafe86f36be59fc22bb02c42b3ff12509c67b53d
2019-04-10 22:31:14 +02:00
James D. Forrester 75a751f1c6 build: Upgrade eslint-config-wikimedia from 0.10.1 to 0.11.0
Change-Id: I36a88cbf8b468646c21d47c3b58433895e8ff31a
2019-04-04 18:01:40 +00:00
Nicholas Ray 799f426f5b Make minerva use updated mediaViewerOverlay factory function
Additional changes:

* rlModuleLoader is no longer needed now that the mediaViewerOverlay is
part of mobile.startup so that was removed in addition to the calls to
it.

* Removed the EVENT_SLIDE/EVENT_EXIT logic as the mediaViewerOverlay
now handles its own slide/exit behavior

Bug: T216198
Depends-On: I36e3c0645c931492fbef0b7c0a49f6a418dbd3a1
Change-Id: I04656c4adb7edc9a111447de3d63044044a8d6dd
2019-03-25 14:18:16 -06:00
Jan Drewniak 6d34e401ff Remove skins.minerva.icons.images.variants module
This module produces 6 icons:

["clock", "profile"] * ["black", "gray", "white"]

of which only 3 are needed: clock-gray, clock-white, profile-gray.

This patch removes the module and moves the "clock" and "profile"
icons into `skins.minerva.icons.images`. Since that module
does not inline SVG's, the additional cost of adding these icons,
which are loaded at startup, is minimal.

The editLocked.svg icon has also been edited to correct its color.

Bug: T218807
Change-Id: Ib54960eb556dfe0c1ae06d2f73d4f350e519e20f
2019-03-22 09:46:14 -07:00
Stephen Niedzielski 88dd2530e7 Update: don't prompt to create User pages
Suppress the redlink drawer for User namespace pages. The redlink drawer
prompts the user to create a missing page but this hinders the usual
workflow for User page visits specifically. A User page is connection to
an account's contributions, age, and other activities and encouraging
the creation of a missing User page when trying to view these
connections is a hindrance, especially if the missing User page is not
associated with the current user.

Bug: T201339
Change-Id: I784493a8ecf28176b5a393cb52d7bfa9fa9b1309
2019-03-19 18:07:32 +00:00
Jan Drewniak 864a1766a7 Refactor pageActions menu to accommodate AMC mode
This patch refactors the markup and JS associated with the page actions
menu in order to achieve greater flexibility in its presentation.
The menu items are now positioned via flexbox and rendered using a mustache
template in PHP.

The goal of this refactor is to accommodate both AMC mode and default
mode with the same markup. No changes should be visible for non-AMC
users with this refactor. No changes to AMC mode have been made in this
patch either.

This patch includes temporary workarounds to avoid problems caused by HTML caching.

Changes include:
- Changing the data structure of the page_actions property in SkinMinerva.php
- Passing that modified data structure into a new mustache template, PageActionMenu.mustache
- Adding new CSS for the new page actions menu HTML
- changing the query selectors in JS to match the new markup
- Making the JS-modified page-actions compatible with the new markup
- Keeping existing CSS and JS to avoid breaking cached HTML

Bug: T213352
Depends-On: I95cf726c4b6d8c3895a26aa6e07f4b1747ee30fe
Change-Id: I5a7d73b20617cb3c6d6379084ac4bea23ec3bc74
2019-02-28 12:31:57 +01:00
Nicholas Ray 7cfcb3641b Refactor LanguageOverlay styles
In I629245100f4ca430a88e450939b90d075e7021f1, languageOverlay styles and
LanguageSearcher styles were split into two different files. This commit
mimics that change in skinStyles. All languageOverlay specific styles
were moved to languageOverlay.less, while all LanguageSearcher styles
were moved to LanguageSearcher.less.

Other chagnes:

* Changed a reference in init.js

Bug: T215657
Change-Id: I395a61cd6051ad5c6b6601205f112b55f5ab8273
2019-02-22 11:02:30 -07:00
Nicholas Ray a48df04a75 Call new language factory instead of LanguageOverlay
When merged with I629245100f4ca430a88e450939b90d075e7021f1, the need to
use rlModuleLoader.js (white screen loader) with LanguageOverlay is
eliminated. When calling the language factory function, an Overlay is
now synchronously returned with a spinner shown in its content area
while the lazy load request + xhr request are executing. PromisedView
will replace this spinner with the LanguageSearcher component when the
promise resolves (rejects continue to be unhandled).

* The getDeviceLanguage function was moved into languageFactory.js
inside MobileFrontend

* the lazy load code and gateway.getPageLanguages code was also moved
into languageFactory.

Bug: T215657
Depends-On: I629245100f4ca430a88e450939b90d075e7021f1
Change-Id: Ie6dad4bd3c80e6cfcc1d7f9ad38941a323ba3cc6
2019-02-21 12:15:55 -08:00
Stephen Niedzielski 9cf8da6514 Hygiene: move PageGateway import to top
Move synchronous import to the top of the file next to other
mobile.startup imports to make it easier to find.

Bug: T215648
Depends-On: I6393e4403034cce51c01003bdf5bcaf3b76ae9f1
Change-Id: Icf52160b84f05a4ac3e069112460cf0bc97a0b4b
2019-02-08 11:59:16 -08: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
Stephen Niedzielski 0dfdbadb4b Hygiene: migrate View.events to constructor prop
Migrate View.events to constructor property. MobileFrontend supports
both the deprecated View.prototype.events and recently passing events as
an argument at construction (see dependent patch).

As a client, Minerva doesn't provide nearly so many extensible classes
as MobileFrontend. The classes changed in this patch are no different so
events are not expected to be extended.

Bug: T208915
Depends-On: Ic193e97b4cd6f72d226bd9768e904bdec168ba39
Change-Id: I68923e2dfd4b2f3e3f3de5185e9b65ab0b665ab7
2019-02-05 22:03:37 +00: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 76be18c788 Tablet code no longer needs to be conditional
The TableOfContents component is now inside mobile.startup
so is readily available.
Thus skins.minerva.tablet.scripts is removed and its code
moved inside skins.minerva.scripts

The check to whether the user is in tablet mode
now happens inside toc.js

The resize handler is dropped on the basis that this is an edge
case that is not worth supporting at the cost of an eventBus.

Depends-On: I353d6d7a6884dae03b55364150c6bb95cd4cc57f
Change-Id: Id16a64573e020b4606fc9b11456163a4cd290e0b
2019-01-14 15:43:50 -08:00
jenkins-bot 0282bb16ca Merge "Composition: DownloadIcon" 2019-01-03 16:41:34 +00: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
Stephen Niedzielski 9f70a004c7 Hygiene: revise CtaDrawer.events to options
Use CtaDrawer.options.events when available instead of CtaDrawer.events.

Change-Id: I7cf5d97bef9179dfa148fa4d5ca447d128031b57
Related: I4a4129b2cac7c7c49559beef0b8780f3211edf9c
Bug: T210870
2018-12-20 19:31:58 +00: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
LukBukkit 9e114731ce
Don't open images if control keys are pressed
This replicates the behavior of the MultimediaViewer on the
desktop when using the mobile page with a desktop browser.

But we have to be careful that this change don't break
other extensions or functions of the skin.

Bug: T206348
Change-Id: Ib4b928eae136d781782dc96e82d653806587292b
2018-11-19 22:10:19 +01:00
Ed Sanders def03877ce Remove obsolete aliases from closures
Bug: T208951
Change-Id: I7cd5ab010596d964fc0896ed250f40d5b6344f6e
2018-11-16 15:16:25 +00:00
jenkins-bot 90629a0578 Merge "Move canEdit checks to server from client" 2018-11-09 22:07:47 +00:00
arcayn c64a4cf811 Move canEdit checks to server from client
Bug: T206262
Change-Id: I08bfc56998f4fb42c8ce76cc2ad9fba83106d476
2018-11-09 21:26:49 +00:00
Bartosz Dziewoński 7bbba14ee1 skins.minerva.scripts/init: Use singleton OverlayManager from MF
Bug: T198765
Depends-On: Ie83846e7dfabe302006d0979f2f21913839e1795
Change-Id: I9c07863ab9c68886d6c5b5f09756e034a4d96477
2018-11-08 16:14:40 +01:00
Bartosz Dziewoński 53a80e1bcc Move edit link enabling/disabling out of skins.minerva.editor
These CSS classes are specific to the Minerva skin and we want to move
this file to MobileFrontend.

Now that this code is all in one place, I noticed that it seems to not
do much at all… Everything in this function looks like it could just
be done in the PHP code. If PHP does it all, then we could remove all
of it for a free performance improvement.

Bug: T198765
Change-Id: I6487c2fc520e14e0856b2e9a6f9dfa5066205817
2018-10-04 14:03:19 -07:00
Bartosz Dziewoński 1a89047d3b Move redlink CTA out of skins.minerva.editor
It is not part of the editor. Redlinks are part of page content.

Bug: T198765
Change-Id: Iee77a228b6f2db1c5f2e85226d6cc1c6a73a89be
2018-09-27 20:13:16 +00:00
jenkins-bot ba827d4b3c Merge "Revert "Minerva is responsible for turning on WikimediaEvents itself"" 2018-09-26 20:42:38 +00:00
Bartosz Dziewoński a773c0eed2 Allow mobile TOC on pages in all namespaces
It was limited to the main namespace since its introduction in
Ieabe8f7071696cde6afbdc6df853aacdb741a4a3. Unfortunately that
commit does not explain the reason.

It should be shown e.g. on user pages (so that sandbox pages
look like the real article) or on project pages (many policy
or help pages on Wikipedia are long and would benefit from it).

It looks like some of the CSS code already assumed that the
TOC would be shown in all namespaces (space for it was reserved
using a 'visibility: hidden;' element on all pages).

Bug: T205312
Change-Id: Id6935f5a7a3701c1c7a38fb37b48b6a3bbc80393
2018-09-25 11:49:25 +02:00
Jdlrobson abf445ec9b Revert "Minerva is responsible for turning on WikimediaEvents itself"
While harmless, this patch is not needed if we take a
different approach as discussed in
https://phabricator.wikimedia.org/T204144#4612790

This reverts commit 01dc9de92e.

Change-Id: Id2fdd8677042121142ab2663edfa0ee400b8bf22
2018-09-24 22:52:12 +00: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
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
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
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