Page MenuHomePhabricator

Split pageIssues.js into smaller functions
Closed, ResolvedPublic5 Story Points

Description

The createBanner code has become overly-complicated and mixes responsibilities. It is hard to read and has bugs.

Solution

In order to isolate the problem outlines in T202349 the createBanner function in pageIssues.js should be split into (at least) 3 functions:

  • createPageIssueBanner - used to create a single page-issue banner
  • createPageIssueBannerMultiple - used to create a page-issue banner the contains multiple issues
  • createPageIssueNotice - used to create the old “this page has issues” treatment.
  • (Not applicable) Unit tests will also need to be updated to reflect these new functions.

These functions should still be called by the createBanner function in order to avoid having to refactoring the initPageIssues function (in this task).

Refactoring this function should help us solve the icon problem described in T202349 by isolating this scenario into one specific function, which will also lessen the chance of regressions while working with this part of the codebase.

Sign off steps

  • (In QA) Un-stall T202349 and queue for estimation.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 5 2018, 10:57 PM

Change 477993 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Refactor pageIssues.js into smaller functions

https://gerrit.wikimedia.org/r/477993

Unfortunelty, the patch above doesn't only separate out functions into smaller pieces. It also (inadvertently) fixes the icon issues raised T202349. This fix includes modifying the way page-issue banners manipulate the DOM, which means the patch should go through thorough browser QA.

Personally, I've tested the patch on the following pages

Catalan

Japanese

English

Russian

Chinese

I took some of these pages from - T204090 QA page issues feature

Assuming this is about MinervaNeue hence adding project tag so this task can be found when searching for tasks related to MinervaNeue.

Jdlrobson triaged this task as Normal priority.Dec 11 2018, 8:08 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 5.
Jdlrobson added a subscriber: Jdlrobson.

Via async estimation

Change 477993 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Refactor pageIssues.js into smaller functions

https://gerrit.wikimedia.org/r/477993

Niedzielski closed this task as Resolved.Dec 17 2018, 2:56 PM
Niedzielski updated the task description. (Show Details)