Page MenuHomePhabricator

[native] Add expand button for drawer elemennts
ClosedPublic

Authored by inka on Nov 24 2022, 6:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 8:57 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 8:51 PM
Subscribers

Details

Summary

Linear issue: https://linear.app/comm/issue/ENG-1881/community-navigation-drawer-on-native
In the comments on this issue it has been decided that we'll use the icons from FontAwesome.

Test Plan

Tested with subsequent diffs. I also tested it by displaying it in RelationshipListItem instead of the PencilIcon.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Nov 24 2022, 6:36 AM
native/navigation/expand-buttons.react.js
9–10 ↗(On Diff #18817)

These values will be revisited during styling
ENG-2343

native/navigation/expand-buttons.react.js
13 ↗(On Diff #18817)

Please remember to always make all of your types $ReadOnly unless you have a strong reason not to

tomek requested changes to this revision.Nov 25 2022, 9:48 AM
tomek added inline comments.
native/navigation/expand-buttons.react.js
16–64 ↗(On Diff #18828)

We have a lot of duplication here. We could make a single component that accepts a prop telling which icon to show and if the touchable is enabled, but that isn't too beneficial for ExpandButtonDisabled case. So instead, how about having two components: one for disabled and one for other cases? This component would have a prop e.g. "expended" | "collapsed" and would choose the icon based on that.

20–22 ↗(On Diff #18828)

Why do have an additional View here?

native/themes/colors.js
158–159 ↗(On Diff #18828)

Should we also add values with these keys to light object?

This revision now requires changes to proceed.Nov 25 2022, 9:48 AM
native/themes/colors.js
84–85 ↗(On Diff #18852)

These values will be revisited during styling ENG-2343

This revision is now accepted and ready to land.Nov 28 2022, 2:20 AM