Page MenuHomePhabricator

[EPIC] Review and refactor MobileFrontend components used by Minerva
Open, HighPublic

Description

Documentation on wiki

https://m.mediawiki.org/wiki/Reading/Web/Projects/Invest_in_the_MobileFrontend_%26_MinervaNeue_frontend_architecture#Review_and_refactor_components

WIP audit of existing MobileFrontend components

https://docs.google.com/document/d/1pxTYAu6tIIPR-8TpUJ5_qKdumj8P76k3cbVo-WJAuzU/edit?usp=sharing

Problems

  • MobileFrontend makes heavy use of inheritance rather than composition - this makes it harder to create new features
  • MobileFrontend uses Hogan.js for templating which is not supported in core and requires bespoke code making it harder to create features that span desktop and mobile.
  • MobileFrontend has a CSS problem - it's very easy to cause UI regressions due to badly organised CSS rules that can cause one component to cause UI regressions in another component.
  • MobileFrontend has no UI component library so it's not easy to verify if all components are rendering correctly.
  • Many of MobileFrontend's codebase has bad code coverage
  • The behaviour of MobileFrontend's components is unpredictable due to the inheritance chain and side effects of events via a global event bus.

What

  • Separation of concerns, separating UI and network and business logic
  • Making our code more easily testable
  • Better unit tests, with less mocking and code units with less scope
  • Using composition instead of inheritance in the UI components
  • Declarative UI rendering
  • Doing away with the direct DOM manipulation paradigm and using a declarative rendering library for the Minerva skin UI components
  • Replacing larger feature specific components with new reusable components

Measuring progress

  • We should monitor the number of files in src/ and resources/ We should expect the number of files in src to increase (and possibly decrease at times) to reflect redundant code being removed
  • We will measure the number of components extending the View class. We hope this will go up as we move away from large multi-concerned components to dumb components.
  • Code coverage will be added during refactoring, so we should report code coverage (at least weekly) to give a sense of how well this is going.
  • We will monitor JS and CSS asset size, which will hopefully decrease as we make our code more reusable.

Measuring success

We will know if we have been successful when:

  • We are capable of building a UI component library using a tool such as storybook
  • It's possible to extract Mobile's components into a well documented external library e.g. T144402

Acceptance criteria

  • All components in MobileFrontend have at most one level of inheritance - e.g. an Overlay extends View.
  • It is clear why the resources folder exists and what files go into it (instead of resources)
  • Code coverage has increased from 50%
  • T94086 has been resolved and it's clear what we are doing with templates going forward.
  • Feature development of an overlay is documented and clear
  • We have made a decision about use of EventBuses (see previous spike in T211724)

Related Objects

StatusSubtypeAssignedTask
Resolved Jdlrobson
Resolved Jdlrobson
OpenNone
DeclinedNone
DeclinedNone
OpenSpikeNone
OpenNone
ResolvedKrinkle
Resolvedovasileva
Resolved Jdlrobson
OpenNone
ResolvedNone
ResolvedNone
ResolvedNiedzielski
Resolvedovasileva
Stalled Jdlrobson
ResolvedBUG REPORT Jdlrobson
Resolvedphuedx
Resolved Jdlrobson
Resolvedovasileva
DeclinedBUG REPORT Jdlrobson
Resolvednray
OpenNone
ResolvedNone
ResolvedNone
Resolved Jdlrobson
Resolved Jdlrobson
OpenNone
DuplicateNone
StalledNone
Resolvedpmiazga
Resolved Jdlrobson
DeclinedNone
Resolved Jdlrobson
Resolved Jdlrobson
DeclinedNone
ResolvedNiedzielski
DuplicateReedy
ResolvedNiedzielski
Resolved Jdlrobson
Resolved Jdlrobson
DuplicateNone
Resolvedpmiazga
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
ResolvedNiedzielski
DeclinedNone
ResolvedNiedzielski

Event Timeline

T195478 aims for 75% code coverage which might require some refactoring to do well. We may want to push that target into this task instead.

ovasileva renamed this task from Review and refactor Minerva components to [EPIC] Review and refactor Minerva components.Jun 4 2018, 10:08 AM
Vvjjkkii renamed this task from [EPIC] Review and refactor Minerva components to 9ccaaaaaaa.Jul 1 2018, 1:08 AM
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
CommunityTechBot renamed this task from 9ccaaaaaaa to [EPIC] Review and refactor Minerva components.Jul 2 2018, 3:52 PM
CommunityTechBot raised the priority of this task from High to Needs Triage.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added a subscriber: Aklapper.
Jdlrobson renamed this task from [EPIC] Review and refactor Minerva components to [EPIC] Review and refactor MobileFrontend components used by Minerva.Jan 17 2020, 10:15 PM
Jdlrobson moved this task from New adventures to Blocked on design on the MinervaNeue board.