[native] Style the navigation drawer (all labels the same)
ClosedPublic
Actions

Authored by inka on Dec 14 2022, 8:20 AM.

Details

Reviewers

ginsu
bartek
tomek

Commits

rCOMM21b1e21d83d3: [native] Style the navigation drawer (all labels the same)

Summary

Linear issues: https://linear.app/comm/issue/ENG-2397/community-drawer-colours
https://linear.app/comm/issue/ENG-2343/style-the-navigation-drawer-on-native
https://linear.app/comm/issue/DES-10/design-for-light-mode-on-native

Designs: https://www.figma.com/file/WmS4u84vnveHHCcZusn5A4/%E2%9A%A1%EF%B8%8F-Mobile-App-Sidedrawer?node-id=4429%3A32903&t=tqg2p8kNEmavnWil-0
https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=3598%3A52104&t=SscXfXRNOzPWSkDp-0

I realised thanks to the light mode designs that drawer item labels on different levels are supposed to be styled differently. This will require a non-trivial amount of changes, so I'll do this in the
next diff. In this diff all labels are styled like the items representing communities.

HitSlop on TouchableOpacity for expand buttons were determined by testing on physical devices.

Screenshot 2022-12-22 at 09.45.48.png (1×508 px, 89 KB)

Screenshot 2022-12-22 at 09.47.00.png (1×506 px, 88 KB)

Test Plan

Run app on multiple simulators and physical devices and see if all looks good.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Dec 14 2022, 8:20 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 14 2022, 8:20 AM

inka added inline comments.Dec 14 2022, 8:22 AM

native/navigation/community-drawer-content.react.js
65 ↗	(On Diff #19343)	SafeAreaView is for avoiding iOS notches.

inka added a parent revision: D5732: [native] Add community drawer navigator.Dec 14 2022, 8:29 AM

inka added a child revision: D5867: [native] Drawer labels styling.

Harbormaster completed remote builds in B14143: Diff 19343.Dec 14 2022, 8:33 AM

inka requested review of this revision.Dec 14 2022, 8:33 AM

I'm not much experienced with styling, but looks OK to me

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

inka added a reviewer: tomek.Dec 20 2022, 3:32 AM

This revision now requires review to proceed.Dec 20 2022, 3:32 AM

It's a good practice to include some screenshot when a styling is affected - could you share some?

native/navigation/expand-buttons.react.js
9–12 ↗	(On Diff #19343)	In native files we aren't using constant case for consts, e.g. in `chat-thread-list-item.react.js` - can we make these consistent?
25–33 ↗	(On Diff #19343)	This doesn't change at all so you can define it outside the component and avoid using memo.

This revision is now accepted and ready to land.Dec 21 2022, 4:55 AM

Address code review

Added screenshots to summary

Harbormaster completed remote builds in B14669: Diff 20008.Dec 22 2022, 1:02 AM

tomek added inline comments.Dec 22 2022, 1:10 AM

native/navigation/expand-buttons.react.js
36 ↗	(On Diff #20008)	Is it possible to use a number instead of a rect?

inka attached a referenced file: F300718: Screenshot 2022-12-22 at 09.47.00.png. (Show Details)Dec 22 2022, 1:14 AM

inka attached a referenced file: F300719: Screenshot 2022-12-22 at 09.45.48.png. (Show Details)

inka added inline comments.Dec 22 2022, 1:21 AM

native/navigation/expand-buttons.react.js
36 ↗	(On Diff #20008)	Not with our current flow types. hitSlop, its types definition Do you want me to see if it can be updated?

tomek added inline comments.Dec 22 2022, 1:31 AM

native/navigation/expand-buttons.react.js
36 ↗	(On Diff #20008)	According to the docs https://reactnative.dev/docs/touchablewithoutfeedback#hitslop it should be possible to use a number. But I think we can postpone updating the types - could you create a task for it?