Page MenuHomePhabricator

[EPIC] Review and refactor Minerva components
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

StatusAssignedTask
OpenNone
OpenNone
OpenNone
StalledNone
OpenNone
OpenNone
OpenNone
ResolvedKrinkle
Resolvedovasileva
ResolvedJdlrobson
OpenNone
ResolvedNone
ResolvedNone
ResolvedNiedzielski
Resolvedovasileva
OpenJdlrobson
ResolvedJdlrobson
Resolvedphuedx
OpenNone
Resolvedovasileva
DeclinedJdlrobson
Resolvednray
StalledNone
ResolvedNone
ResolvedNone
OpenNone
OpenNone
DuplicateNone
OpenNone
Resolvedpmiazga
ResolvedJdlrobson
DeclinedNone
OpenNone
ResolvedJdlrobson
DeclinedNone
ResolvedNiedzielski
DuplicateReedy
ResolvedNiedzielski
ResolvedJdlrobson
ResolvedJdlrobson
DuplicateNone
Resolvedpmiazga
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
ResolvedNiedzielski
OpenNone
OpenEdtadros

Event Timeline

Jhernandez updated the task description. (Show Details)May 24 2018, 12:28 PM

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
Jdlrobson moved this task from Backlog to New adventures on the MinervaNeue board.Jun 13 2018, 3:57 PM
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 triaged this task as High priority.Jul 9 2018, 4:30 PM
Jdlrobson updated the task description. (Show Details)Dec 20 2018, 10:44 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Feb 6 2019, 12:45 AM
Jdlrobson updated the task description. (Show Details)Feb 12 2019, 11:06 PM
Niedzielski changed the status of subtask T207770: Split PageGateway from Stalled to Open.Apr 8 2019, 3:41 PM