Page MenuHomePhabricator

<maplink> without <mapframe> still loads leaflet libs on page load
Closed, ResolvedPublic

Description

Tested at http://en.wikipedia.beta.wmflabs.org/wiki/MapLink

That page only has <maplink>, but it still loads the mapbox library on initial page load. Instead, it should load it only after the user clicks.

Looking at the dependencies, ext.kartographer.live is what contains all the code, and ext.kartographer.fullscreen gets loaded on maplink click, and it does not depend on the ext.kartographer.live. The MapLink::render() adds the ext.kartographer.live for some reason - added by ec8ef2f1b2da02e1944252cd56d462e59f16de7d (@MaxSem?)

Proposed Structure

module Added by PHP depends onmay load via JSnotes
maplinkon <maplink>fullscreenContains a tiny JS loader that attaches to maplinks. CSS to style links.
mapframeon <mapframe>site, livefullscreenContains JS loader that modifies to mapframes. CSS to style mapframe.
mapframe.snapon <mapframe>mapframeFor later - when we implement map snapshot image service, inseart this instead of mapframe
fullscreensite, liveLoaded by maplink and mapframe
livemapbox, settingsThe core map resource, maybe used by external tools like Commons upload wizard
siteLoaded only for <mapframe>. Custom class
settingsCustom class, sets global map configuration JS vars
mapbox (lib)External lib, includes Leaflet
visualEditormaplink, mapframe, ve.dialogMain Visual Editor resource. TBD: At what point should maplink and mapframe resources be loaded?
ve.dialogleaflet.drawVE code for map editor dialog
leaflet.draw (lib)mapboxExternal lib - drawing tools for VE

Existing structure

PHP:

TagHandler.phpaddModuleStyles( 'ext.kartographer.style' )
MapFrame.phpaddModules( 'ext.kartographer.live' )
MapLink.phpaddModules( 'ext.kartographer.live' )

JS:

kartographer.jsopenFullscreenMapext.kartographer.fullscreen
ve.ce.MWInlineMapsNode.jsconstructorext.kartographer.settings
ve.ui.MWMapsDialog.jsconstructorext.kartographer.editor
ve.ce.MWMapsNode.jsconstructorext.kartographer.settings
ve.ce.MWMapsNode.jsupdate(): if requiresInteractiveext.kartographer.live

Modules:

moduleused bydependenciesstylescriptnotes
ext.kartographer.settingsjs-veSet JS configuration for Kartographer, like available scaling factors, styles, etc. Used by live & visualeditor resources, loaded by VE ve.ce.MWInlineMapsNode & ve.ce.MWMapsNode
ext.kartographer.stylephp-allstyles/kartographer.lessStyles for maplink, mapframe, fullscreen (TODO: maybe we should break it into separate modules). Loaded by mapframe & maplink tag handlers
ext.kartographer.siteMediaWiki:Kartographer.cssMediaWiki:Kartographer.jsSite-customized css & js, should only be loaded for <mapframe>, fullscreen, and edit. Used by live & fullscreen resources
ext.kartographer.livephp-all, js-ve*mapbox, ext.kartographer.settings, ext.kartographer.sitelib/leaflet.sleep.js, kartographer.jsShow interactive map - required by <mapframe>, fullscreen, editor. Used by editor resource. Loaded by all tags and ve.ce.MWMapsNode
ext.kartographer.fullscreenjsext.kartographer.sitekartographer.MapDialog.jsLoaded when user clicks on a maplink or expands a mapframe, from JS in openFullscreenMap()
ext.kartographer.editorjs-veleaflet.draw, ext.kartographer.liveLoaded by VE map editor dialog - ve.ui.MWMapsDialog
ext.kartographer.visualEditorext.kartographer.settingsve.ui.MWMaps.cssve-maps/*Loads css & js for the VE map support. Used only by VisualEditorPluginModules setting below
mapboxlib/mapbox/*lib/mapbox/*External mapframe lib, handles drawing of the map. Used by live and leaflet.draw resources
leaflet.drawmapboxlib/leaflet.draw/*lib/leaflet.draw/*External leaflet lib to allow visual geojson editing in the VE map dialog, loaded by editor resource

Details

Related Gerrit Patches:
mediawiki/extensions/Kartographer : masterFirst step cleaning up resources
mediawiki/extensions/Kartographer : masterSplit the JS codebase into several modules.

Event Timeline

Yurik created this task.Apr 30 2016, 9:00 PM
Restricted Application added a project: Discovery. · View Herald TranscriptApr 30 2016, 9:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Yurik renamed this task from Performance: <maplink> without <mapframe> still loads leaflet libs on page load to <maplink> without <mapframe> still loads leaflet libs on page load.May 1 2016, 12:40 AM
Yurik added a project: Performance Issue.
Yurik moved this task from Unsorted to General on the Maps (Kartographer) board.
MaxSem moved this task from Backlog to To-do on the Maps-Sprint board.May 27 2016, 10:13 PM
Yurik claimed this task.May 28 2016, 6:25 PM
Yurik moved this task from To-do to In progress on the Maps-Sprint board.

Change 291568 had a related patch set uploaded (by Yurik):
First step cleaning up resources

https://gerrit.wikimedia.org/r/291568

Yurik updated the task description. (Show Details)May 29 2016, 1:48 AM
Yurik updated the task description. (Show Details)May 29 2016, 2:34 AM
Yurik updated the task description. (Show Details)May 29 2016, 2:52 AM
Yurik updated the task description. (Show Details)
Yurik updated the task description. (Show Details)May 29 2016, 2:59 AM
Yurik updated the task description. (Show Details)May 29 2016, 8:30 AM
Yurik updated the task description. (Show Details)May 29 2016, 9:04 AM
Yurik updated the task description. (Show Details)May 29 2016, 9:36 AM
Yurik edited projects, added Proposal; removed Patch-For-Review.
Yurik added a subscriber: Esanders.
Yurik updated the task description. (Show Details)May 29 2016, 7:53 PM

Change 295599 had a related patch set uploaded (by JGirault):
Split the JS codebase into several modules.

https://gerrit.wikimedia.org/r/295599

Change 295599 merged by jenkins-bot:
Split the JS codebase into several modules.

https://gerrit.wikimedia.org/r/295599

Yurik closed this task as Resolved.Jun 27 2016, 9:49 PM
Yurik reopened this task as Open.

@JGirault could you copy the table describing each resource to the kartographer's documentation page? I'm sure it will need some updating. Thanks! (keeping it open until then - we don't want to loose that info)

Yurik reassigned this task from Yurik to JGirault.Jun 27 2016, 9:50 PM

@Yurik Do you see more work required, or do you think the current state of the code resolves this ticket?

Change 291568 abandoned by Yurik:
First step cleaning up resources

https://gerrit.wikimedia.org/r/291568