Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- [Base] - Upgrade Left drawer #187

Merged
merged 6 commits into from
Jun 23, 2016
Merged

- [Base] - Upgrade Left drawer #187

merged 6 commits into from
Jun 23, 2016

Conversation

fvonhoven
Copy link
Contributor

Ignite Base Change

  • Works on iOS/Android/XDE
  • Tests Pass (run: npm run test)
  • Code is StandardJS compliant (run: npm run lint)
  • First step in establishing a general best practice for a Menu Drawer that is responsible for ancillary navigation

Typically a Menu Drawer can be responsible for navigating to screens when deep into the navigation stack from the "Home Screen." Here we've set up a lean component that's business logic is handled at the highest level (Root) that is on par with the navigator - where it should be IMO. This allows the Drawer Content Component to be a "dumb" stand-alone component that functions without any knowledge of state, navigator, and dispatch.

Since we wrap the Navigator with a Drawer

screen shot 2016-06-23 at 11 37 22 am

This upgrade method eliminates the pesky issue of passing around navigator and dispatch to the Menu Drawer, which are necessary when dealing with navigation, as well as consequential async problems that arise from navigator handling during the component and app lifecycle.

alt tag

YAY!!

How's it work?

DrawerContent

  • Created a DrawerContent component that will live in the Drawer at Root
  • DrawerContent's parent is a ScrollView to allow for many items (buttons, etc.)
  • Also created a default DrawerButton that has settable text and an onPress function
  • Load up your DrawerContent render method with as many DrawerButtons as you like - even throw in your favorite logo 😉

screen shot 2016-06-23 at 11 11 31 am

  • We're going to bind onPresss' called function inside a constructor method, because binding inside the onPress property creates a new function in the render method, which causes react to reset that property - performing “work” where none is needed. (Thanks @skellock !)

screen shot 2016-06-23 at 11 23 34 am

  • Our DrawerComponent is passed a required onPushRoute property from Root

screen shot 2016-06-23 at 11 32 44 am

  • By passing DrawerContent this function, we've allowed for navigator and dispatch to remain in Root and still be accessed by our DrawerContent component

Root

  • Our DrawerContent component's onPushRoute property calls handlePushRoute (which is also bound in a constructor method in Root
  • handlePushRoute pushes the route AND closes our Menu Drawer

screen shot 2016-06-23 at 11 40 43 am

Bing, Bang, alt tag

@skellock
Copy link
Contributor

Thx @fvonhoven . I think much of this will get simplified even further once @kevinvangelder brings in react-native-router-flux which relies on @GantMan 's upcoming RN 0.28 upgrade.

Thanks for taking the time to post a detailed explanation sir.

@skellock
Copy link
Contributor

For more information on why we shouldn't bind inside render:

https://medium.com/@esamatti/react-js-pure-render-performance-anti-pattern-fb88c101332f#.2t3vjohs5

Scroll to the section that says "Functions create identities too".

Then scroll back up to the top & read the rest of the article because it's amazing. Thanks @epeli.

@GantMan
Copy link
Member

GantMan commented Jun 23, 2016

hah

@GantMan GantMan merged commit 24c7f3c into master Jun 23, 2016
@GantMan GantMan deleted the upgrade-drawer branch June 23, 2016 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants