Page MenuHomePhabricator

[web] Fix drawer higlight glitch
ClosedPublic

Authored by inka on Feb 9 2023, 2:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:05 PM
Unknown Object (File)
Wed, Mar 20, 9:04 PM
Unknown Object (File)
Tue, Mar 12, 3:50 PM
Subscribers

Details

Summary

In D6292 I made drawer items react based on the current tab, by having them fetch a Handler component type and get the right handler for the selected tab. In D6389 I added a isActive field to
the
handler object they obtain, that is used for determining which item is supposed to be higlighted. Then in D6363 I decided to have the drawer render two drawer item lists, and have one of them be
hidden, based on the current tab (two, becuse we have the same behaviour for all tabs but the Calendar tab). This means, that the items don't need to change the handler they use, because the list of
items is changed instead. Instead, they should get a Handler type from their parent, and always use the same one.

Not doing this was causing glitches when switching tabs

Test Plan

Rub web app, check that drawer items in different tabs are highlighted properly, and there is no glitching.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka edited the test plan for this revision. (Show Details)
inka requested review of this revision.Feb 9 2023, 2:33 AM

Fix error when expanding items

web/sidebar/community-drawer-item.react.js
20 ↗(On Diff #22288)

Nit: typical pattern I've seen for this sort of thing is to keep Handler lowercase until the place in the code where you use it as a component. Then at that place, you can do something like:

const Handler = props.handler;
tomek added inline comments.
web/sidebar/community-drawer.react.js
49–52 ↗(On Diff #22288)

This value never changes and is independent from the props, so we can consider it extracting from the component

This revision is now accepted and ready to land.Feb 10 2023, 2:11 AM