Page MenuHomePhabricator

[web] Remove unused code
ClosedPublic

Authored by inka on Jan 4 2023, 3:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 3, 9:31 PM
Unknown Object (File)
Wed, Apr 3, 9:31 PM
Unknown Object (File)
Wed, Apr 3, 9:31 PM
Unknown Object (File)
Wed, Apr 3, 9:31 PM
Unknown Object (File)
Wed, Apr 3, 9:30 PM
Unknown Object (File)
Wed, Apr 3, 9:13 PM
Unknown Object (File)
Tue, Apr 2, 12:39 AM
Unknown Object (File)
Tue, Apr 2, 12:39 AM
Subscribers

Details

Summary

The removed styles were being overwritten in all cases by components underneath, but caused problems for me when I was trying to add community drawer.

Test Plan

Run web app, checked that removing these styles makes no difference in inbox and in settings.

  1. checked in all files that import left-layout-aside.css which of them use the container style, and container style from left-layout-aside.css is only used in left-layout-aside.js
  2. container style is used on the <aside> element, and the styles I removed concern its foreground objects and svgs, so I check its children:
  3. its children are CommunityPicker, SettingsSwitcher, AppSwitcher
  4. CommunityPicker: the outer div returned from community picker has a style that overrides svgs colours and padding. CommunityPicker has no other foreground objects than the two svgs: inbox and setting icons. So the styles I removed don’t concern CommunityPicker.
  5. SettingsSwitcher displays two items inside of the NavigationPanel. The active item gets colour from NavigationPanel. And the non-active tab gets its colour in SettingsSwitcher:

https://github.com/CommE2E/comm/blob/master/web/sidebar/settings-switcher.react.js#L26
https://github.com/CommE2E/comm/blob/master/web/sidebar/settings-switcher.react.js#L45
https://github.com/CommE2E/comm/blob/master/web/sidebar/left-layout-aside.css#L28

  1. AppSwitcher: displays three items, and does the same thing as SettingsSwitcher
  2. There are no deeper nested children in any of those three children of the <aside> element.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Jan 4 2023, 3:46 AM

The removed styles were being overwritten in all cases by components underneath

Could you modify the Test Plan to show how you confirmed this, eg. git grep results or something

Agree. We need to look on all files that import that .css file and look if those styles aren't used anywhere.

inka edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Jan 18 2023, 2:06 AM
This revision was automatically updated to reflect the committed changes.