Improving category overlay
Homework: Everyone to review the CategoryOverlay code (https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/mobile.categories.overlays/CategoryOverlay.js) and related CategoryGateway (but not CategoryAddOverlay.js) and think about how they would write it given a magical imaginary architecture (e.g. if we're using React). On 9th April we will talk about this code and pain points and share what we've learned. The goal would be to identify a rewrite task with clear goals and an outline of first steps to a refactor. …
What is the category overlay
The category overlay is a beta feature on mobile wikipedia that shows a full-screen overlay of page categories. The categories are divided into two lists "content based" and "organizational" under a set of tabs. Clicking the tab switches the list, and clicking a category brings you to the category page.
What does it do?
The category overlay does the following things:
- Registers with
OverlayManagerto close/open/render the overlay.
CategoryGatewayto fetch the list of categories
Overlayin order to populate an ovelay instance with specific content
- Creates tabs to switch between category lists
- Creates the category lists
infiniteScrollto lazy-load more elements from
- Manages the
inifiteScrollloading & behaviour
Can we identify discreet components based on this functionality?
- The overlay itself
- List + list items (for what is an empty list?)
- Infinite Scroller
In React land, the component tree could be expressed like this:
<Overlay> <Tabs> <TabHeaders labels="['content-based', 'organizational']"/> <TabContent> <Panel for="activeTab()" with="infinteScroll()"> <CategoryList> <CategoryListItem/> </CategoryList> </Panel> </TabContent> </Tabs> </Overlay>
Points of discussion.
In the component tree above, there is only one content panel, but two tabs. This assumes the ability to switch the DOM of the
<Panel> component based on the active-state of the tab headers.
Infinite scrolling - How is scrolling handled when there are actually two different scrollable lists? When you scroll the "content-based" category list, does the "organizational" category list get updated too? Or, should these be two seperate scrollers? Does that mean two different API requests?
Misc - The CategoryOvelay defines a
category-added event but doesn't actually use it, can we better document these kinds of shared events.
What about the gateway?
First of all, CategoriesGateway extends SearchGateway? why?
- Can't find any methods in CategoryGateway that are reused from SearchGateway
- Doesn't seem to take advantage of the
_cachefunction in SearchGateway
Why is this is inheriting from SearchGateway and then extending it's prototype?
- Oh this is why because
OO.inheritClassoverrides the prototype
Beware: This redefines the prototype, call before setting your prototypes.
- If seach gatway is needed, why not just call
CategoryGateway.parent.applyin the constructor? and then use
OO.mfExtendlike everywhere else?
- Oh this is why because
The use of
util.extendto add a parameter in the search method is confusing, because
util.extendis so frequently used to extend classes.
I see a
then, but no
I see a jQuery.Deferred in the docs, is it really?
Similaries with other components
The category overlay manages the inifinite scroller in a very similar way to Special:Uploads (and maybe others). Components have to manually create and decide when to show/hide the spinner, seems like this could be managed by the infiniteScroller itself. Triggering a reload also has to be invoked manually by the component, can this be managed by the infinite scroller by triggering an event instead?
The CategoryGateway processes data in a similar way as the SearchGateway. It has to check if the response contained
data.query.pages (or something) or if it's empty or contains an error. The Gateways should probably handle errors and serve the components just the data they need to do their thing, so instead of
data.query.pages the gateway should return just
pages or an empty array (or some other convention if the response is empty, return
Can we abstract the common handling into a generic Gateway component? seems like request caching could be utilized by many components, as well as error handling and
continue handling. Something like a
.next() method in the case of
canContinue would be nice.