Page MenuHomePhabricator
Create Task
Maniphest T211828

MFA: Drop all usages of isBorderBox and className props on View and remove log warnings
Closed, ResolvedPublic3 Estimated Story Points
Actions

Description

In T209007 we started the deprecation process for className and isBorderBox properties on our View classes.

The warning Use of "className" is deprecated. Setting className on the View is deprecated. Please use options. now shows for many workflows.

This task completes the work by dropping those warnings and then restricting className and isBorderBox to be options only.

This work puts us on course for making greater use of composition in our codebase.

Acceptance criteria

  • className is always passed to the parent constructor rather than set as a property
  • isBorderBox is always passed to the parent constructor rather than set as a property
  • If a class extends View and defines className and isBorderBox these have no impact
  • No deprecation notices can be shown in the console under any circumstances

Developer notes

This is a 2 patch job - 1 to clean up the deprecation warnings, 1 to remove the root of the console.logs

Details

SubjectRepoBranchLines +/-
notificationsFilterOverlay: Pass className as propmediawiki/extensions/MobileFrontendmaster+8 -7
Break the View property contract for isBorderBox and classNamemediawiki/extensions/MobileFrontendmaster+11 -96
Suppress deprecation warnings on various Viewsmediawiki/extensions/MobileFrontendmaster+54 -67
Customize query in gerrit

Related Objects

Event Timeline

Jdlrobson created this task.Dec 12 2018, 10:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 12 2018, 10:28 PM
Jdlrobson renamed this task from Drop all usages of isBorderBox and className props on View and remove log warnings to MFA: Drop all usages of isBorderBox and className props on View and remove log warnings.Dec 12 2018, 10:28 PM
Jdlrobson triaged this task as High priority.
Jdlrobson added a project: MobileFrontend (MobileFrontend and MinervaNeue architecture).
Jdlrobson mentioned this in T209007: MFA: View: Make isBorderBox and className passable options to the View class to reduce reliance on inheritance.Dec 12 2018, 10:34 PM
gerritbot subscribed.Dec 12 2018, 10:42 PM

Change 479344 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Suppress deprecation warnings on various Views

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

gerritbot added a project: Patch-For-Review.Dec 12 2018, 10:42 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Dec 12 2018, 11:09 PM
Jdlrobson set the point value for this task to 3.Dec 14 2018, 6:10 PM
Jdlrobson edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2); removed Web-Team-Backlog.

(via async estimation)

Jdlrobson moved this task from To Do to Doing on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 17 2018, 9:26 PM
Jdlrobson moved this task from Doing to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 17 2018, 10:01 PM
Jdlrobson updated the task description. (Show Details)
gerritbot added a comment.Dec 17 2018, 10:03 PM

Change 480251 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Break the View property contract for isBorderBox and className

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

Jdlrobson added a subscriber: • Niedzielski.Dec 17 2018, 10:03 PM

@Niedzielski the above patches might help with your work on T210870

gerritbot added a comment.Dec 18 2018, 7:28 PM

Change 479344 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Suppress deprecation warnings on various Views

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

Jdlrobson moved this task from Needs Code Review to Needs More Work on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 18 2018, 7:29 PM
ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.12; 2019-01-08).Dec 18 2018, 8:00 PM
Jdlrobson moved this task from Needs More Work to Blocked on Others on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 19 2018, 5:45 PM

Waiting on output of T210870

Jdlrobson moved this task from Blocked on Others to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 20 2018, 6:02 PM
gerritbot added a comment.Dec 20 2018, 7:37 PM

Change 480251 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Break the View property contract for isBorderBox and className

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

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 20 2018, 9:41 PM
phuedx claimed this task.Dec 21 2018, 3:06 PM
phuedx updated the task description. (Show Details)
phuedx updated the task description. (Show Details)
phuedx updated the task description. (Show Details)Dec 21 2018, 3:10 PM

className is always passed to the parent constructor rather than set as a property

[[ https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/mobile.notifications.overlay/NotificationsFilterOverlay.js#L42-L46 | This is not true for mobile.notifications.filter.overlay/NotificationsFilterOverlay ]].

phuedx updated the task description. (Show Details)Dec 21 2018, 3:11 PM
phuedx added a comment.Dec 21 2018, 3:15 PM

isBorderBox is always passed to the parent constructor rather than set as a property

Confirmed with ag --before=3 --after=3 --js isBorderBox src/ resources/mobile.*.

phuedx updated the task description. (Show Details)Dec 21 2018, 3:17 PM

If a class extends View and defines className and isBorderBox these have no impact
No deprecation notices can be shown in the console under any circumstances

These were both done in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/480251/. The first was done by removing the _resolveInheritedProp function and its usage to resolve the className property. The second was done by setting the DEPRECATED_PROPERTIES constant to [].

gerritbot added a comment.Dec 21 2018, 3:35 PM

Change 481181 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/MobileFrontend@master] notificationsFilterOverlay: Pass className as prop

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

phuedx moved this task from Ready for Signoff to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 21 2018, 3:36 PM

@Jdlrobson: I believe that https://gerrit.wikimedia.org/r/481181 should allow us to mark the first AC as done so that we can resolve this task.

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 21 2018, 5:30 PM
gerritbot added a comment.Dec 21 2018, 5:43 PM

Change 481181 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] notificationsFilterOverlay: Pass className as prop

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

phuedx updated the task description. (Show Details)EditedDec 21 2018, 5:49 PM

className is always passed to the parent constructor rather than set as a property

Confirmed with ag --before=3 --after=3 --js className src/ resources/mobile.*.

phuedx closed this task as Resolved.Dec 21 2018, 5:50 PM

Being bold.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits