The concept of a "page model" is a common subject in MobileFrontend. It may be its most significant model. It should be easy to create, reason about, and manipulate such an important model.
However for historic reasons the Page is also a View (it used to be used to re-render a page after an edit). It doesn't need to be, in fact, a page is never rendered, it is only ever passed around. As a result, I recommend we make this a pure model.
Acceptance Criteria
- Before writing any patches have a slack conversation outlining your plans to avoid the problems we've been hitting in our refactor work and ensure you have buy in. Make sure to share your work early and check in that everyone understands it.
- Page / Page.test.js is a plain model. No or very minimal work in its constructor except assignment. No inheritance. There will no decrease in the amount of existing code or major API refactors so any model methods that existed previously will still be there.
- Pull out PageJSONParser and PageJSONParser.test.js which parses JSON and returns a Page model. It clarifies in the comments what gateways or response formats it works with given the flexibility of the MW API. It exists in a distinct file.
- The preRender is removed/renamed from the Page class to avoid confusion.
Developer notes
- Avoid renames - they are not worth the hassle - simply work towards removing the View inheritance
- All construction parameters including those passed via "options.prop" are well documented with JSDocs including typing and whether parameters are optional.
PageJSONParser looks like the easiest to tackle and may be a good place to start. PageView should probably come next so that it's easy to test the Page model without a DOM.
Sign off notes
The Page object is used in various places, so breaking changes to its API are going to be tricky and risky and should be avoided. As part of completing this task, follow up work should be proposed in the comments and follow up tasks created as part of sign off
QA instructions
This is one of those tickets where QA becomes 🙏 because it touched so many different parts of the mobile site. Here are probably the main ones to look out for and it might be helpful to keep the JS console open as you are browsing the site and see if you notice any out of the ordinary JS errors:
Editing
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain and make sure you can edit using the visual editor and source editor while logged in and anonymous. I'm not sure if tests rely on this page or not so you might want to undo any of the edits you make to preserve its original text.
SearchOverlay
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain while anonymous or logged in (shouldn't matter) and click on the search box. Search for anything making sure that the list populates like normal and that you are able to click on an item and go to that page.
Watchstar/Watchlist
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain while logged in and click on the watchstar icon to add it to your watchlist.
- Click on the hamburger menu to open the main menu and click the Watchlist link and verify the page shows up in that list
- Go back to https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain while logged in and click on the watchstar icon to remove it from your watchlist.
- Click on the hamburger menu to open the main menu and click the Watchlist link and verify the page is removed from the list
Table of contents/section toggling
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama while anon or logged in (shouldn't matter) and make sure the table of contents toggling works.
- Click on a section and verify that the section toggling works as well
Reference Drawer
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama while anon or logged in (shouldn't matter) and click on a reference link. Make sure the black drawer appears with the reference
Page issues
- Go to https://en.m.wikipedia.beta.wmflabs.org/wiki/Pharmacovigilance while anon (logged in is doing weird stuff with new user page) and verify that you see page issues present. Also verify that clicking on the "Learn more" link on one of them opens up the page issue overlay.
QA Results
| AC | Status | Details |
|---|---|---|
| 1 | ✅ | T193077#5363782 |
| 2 | ✅ | T193077#5363782 |
| 3 | ✅ | T193077#5363782 |
| 4 | ✅ | T193077#5363782 |
| 5 | ✅ | T193077#5363782 |
| 6 | ✅ | T193077#5363782 |
