Page MenuHomePhabricator

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

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

Event Timeline

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.

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

Jdlrobson set the point value for this task to 3.Dec 14 2018, 6:10 PM

(via async estimation)

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

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

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

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

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

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

This is not true for mobile.notifications.filter.overlay/NotificationsFilterOverlay.

phuedx updated the task description. (Show Details)Dec 21 2018, 3:11 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 [].

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

@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.

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.