Page MenuHomePhabricator
Differential D5533

[native] Refactor app-navigator.js
ClosedPublic
Actions

Authored by inka on Nov 4 2022, 7:01 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:38 PM
Unknown Object (File)
Thu, Apr 11, 10:36 PM
Unknown Object (File)
Wed, Apr 3, 2:43 AM
View All 69 Files
Subscribers
• abosh
ashoat

Details

Reviewers
tomek
atul
ginsu
bartek
Commits
rCOMM8082920bcab0: [native] Refactor app-navigator.js
Summary

tab navigator and overlay navigator should be in separate files.
Linear issue: https://linear.app/comm/issue/ENG-1883/refactor-app-navigatorjs-so-each-navigator-has-its-own-file

Test Plan

run ios simulator

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka created this revision.Nov 4 2022, 7:01 AM
Herald added subscribers: • abosh, ashoat. · View Herald TranscriptNov 4 2022, 7:01 AM
Harbormaster completed remote builds in B13172: Diff 18077.Nov 4 2022, 7:12 AM
inka requested review of this revision.Nov 4 2022, 7:12 AM
ginsu accepted this revision.Nov 4 2022, 8:22 AM

tested locally, and everything works well!

This revision is now accepted and ready to land.Nov 4 2022, 8:22 AM
inka added 1 blocking reviewer(s): tomek.Nov 7 2022, 1:07 AM
This revision now requires review to proceed.Nov 7 2022, 1:07 AM
tomek requested changes to this revision.Nov 7 2022, 1:39 AM
tomek added inline comments.
native/navigation/tab-navigator.react.js
79 ↗(On Diff #18077)

Is it a correct route name?

This revision now requires changes to proceed.Nov 7 2022, 1:39 AM
inka added inline comments.Nov 7 2022, 5:19 AM
native/navigation/tab-navigator.react.js
79 ↗(On Diff #18077)

No, sorry

inka updated this revision to Diff 18106.Nov 7 2022, 5:21 AM

Fix route name

Harbormaster failed remote builds in B13199: Diff 18106!Nov 7 2022, 5:29 AM
tomek accepted this revision.Nov 7 2022, 6:38 AM
This revision is now accepted and ready to land.Nov 7 2022, 6:38 AM
bartek accepted this revision.Nov 7 2022, 6:40 AM
ashoat added inline comments.Nov 7 2022, 7:02 AM
native/navigation/tab-navigator.react.js
78–83 ↗(On Diff #18106)

Does this allow us to remove the eslint-disable?

inka added inline comments.Nov 7 2022, 7:26 AM
native/navigation/tab-navigator.react.js
78–83 ↗(On Diff #18106)

No, because then Flow doesn't allow us to use this as a Screen in AppNavigator, because "(...) property navigation is missing (...)" / "(...) property route is missing (...)"

ashoat added inline comments.Nov 7 2022, 8:05 AM
native/navigation/tab-navigator.react.js
78–83 ↗(On Diff #18106)

Got it, thanks @inka!

inka updated this revision to Diff 18142.Nov 7 2022, 8:26 AM

Rebase

Harbormaster failed remote builds in B13218: Diff 18142!Nov 7 2022, 8:30 AM
inka updated this revision to Diff 18174.Nov 8 2022, 12:41 AM

Rebase

Harbormaster completed remote builds in B13246: Diff 18174.Nov 8 2022, 12:55 AM
Closed by commit rCOMM8082920bcab0: [native] Refactor app-navigator.js (authored by • InkaSokolowska). · Explain WhyNov 8 2022, 2:16 AM
This revision was automatically updated to reflect the committed changes.
• InkaSokolowska added a commit: rCOMM8082920bcab0: [native] Refactor app-navigator.js.

Revision Contents
Changeset List

PathSize
native/
calendar/
2 lines
2 lines
chat/
2 lines
2 lines
settings/
2 lines
navigation/
117 lines
131 lines

Diff 18177

View Options

native/calendar/calendar.react.js

Loading...
View Options

native/calendar/entry.react.js

Loading...
View Options

native/chat/chat-list.react.js

Loading...
View Options

native/chat/chat-thread-list.react.js

Loading...
View Options

native/chat/settings/thread-settings.react.js

Loading...
View Options

native/navigation/app-navigator.react.js

Loading...
View Options

native/navigation/tab-navigator.react.js

Loading...