There are many cases where local functions or variables are being exposed as part of the mw.popups object. We should go over the codebase and make those functions and variables local.
In a follow up can we take the opportunity to stop clickHandler and leaveInactive from being a global?
It's really confusing as it's only used in this file so no need to be exposed anywhere.
- Variables that can be local are not made global.
Notes on the use of globals
The following globals encourage the renderer to contain logic and to act like a controller (getting the information) and with some refactoring do not need to be global. Many are also exposed for testing purposes. We should reconsider what, why and how we test these things
- mw.popups.removeTooltips - exposed for testing but shouldn't be a global
- mw.popups.setupTriggers - feels like this is part of an init method.
- mw.popups.getRandomToken - currently exposed by ext.popups.core but only used by ext.popups.schemaPopups.utils
- mw.popups.getPreviewCountBucket - currently exposed by ext.popups.core but only used by ext.popups.schemaPopups.utils
- mw.popups.incrementPreviewCount - currently exposed by ext.popups.core but only used by ext.popups.renderer.desktopRenderer
This should probably be moved to ext.popups.settings.js or their own dedicated module. isUserInCondition is only exposed for testing purposes.
All the following can be trivially changed:
This appears to be exposed for the purpose of testing and for use inside ext.popups.schemaPopups We could use module.exports instead to avoid population of the global mw.popup and explore merging the two ResourceLoader modules.
Probably shouldn't expose any globals yet it exposes the following:
It overrides mw.popups.render.renderers.article. it would be better to do something explicit such as mw.popups.registerRenderer
openPopup and closePopup should arguably be the only globally exposed methods.