Page MenuHomePhabricator

[web] Change drawer items styling to allow full-width highlights
ClosedPublic

Authored by inka on Jan 18 2023, 4:48 AM.
Tags
None
Referenced Files
F3379829: D6294.id22443.diff
Wed, Nov 27, 7:30 PM
F3379807: D6294.id21036.diff
Wed, Nov 27, 7:22 PM
F3379796: D6294.id22531.diff
Wed, Nov 27, 7:16 PM
F3379793: D6294.id21080.diff
Wed, Nov 27, 7:14 PM
F3379692: D6294.id21077.diff
Wed, Nov 27, 6:30 PM
F3379686: D6294.id21075.diff
Wed, Nov 27, 6:27 PM
Unknown Object (File)
Sun, Nov 24, 1:39 AM
Unknown Object (File)
Sun, Nov 24, 1:15 AM
Subscribers

Details

Summary

Issue: https://linear.app/comm/issue/ENG-2738/highlight-drawer-item
Designs this is needed for: https://linear.app/comm/issue/DES-3/designs-for-side-drawer-community-navigation-on-web,
https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=7284%3A146198&t=CoLi2QV03acY7gv2-4
Before, drawer items had a margin left of 16, and all their children were being indented with them. So each next level had a margin equal to the margin of its parent + 16. But because we will want
to highlight drawer items, and the highlight is supposed to be almost as wide as the drawer, that approach will no longer work.
I will also not take the approach that I took for the styling of labels - where the styling was being determined in CommunityDrawerContent and passed down in the itemData prop, becaue the function
that is used to create itemData is used on both native and web, and native doesn't have a need for this change. Instead I'll have the drawer items take a padding prop. Each item will then pass its
padding value + 12 (not 16 because the designs have changed) to its children.
I don't love this solution, but it doesn't make native dependent on things it doesn't need, and works without requiring changes if we decide to increase the number of levels.

I highlighted an item to show how it is rendered now, but obviously without the colour:

image.png (412×642 px, 33 KB)

Test Plan

Run the web app, check that drawer items display correctly.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 18 2023, 4:52 AM
Harbormaster failed remote builds in B15427: Diff 21036!
inka requested review of this revision.Jan 19 2023, 1:57 AM

Change indentation as has been decided in the designs task

tomek added inline comments.
web/sidebar/community-drawer-item.react.js
104 ↗(On Diff #21077)

Maybe rename it to style?

This revision is now accepted and ready to land.Jan 19 2023, 4:32 AM