Tab navigator experiments #19

vnovick · 2018-12-30T21:04:03Z

Nested Tab Navigator example
Experiments with navigations
Changes for all reactElement return types to reactClass

…experiment

package.json

czystyl · 2018-12-31T06:29:54Z

src/TabNavigator.re

    navigationOptions: screenOptions,
  };

+  module NavigationProp = {
+    [@bs.send] external navigate: string => unit = "navigate";


Do we pass the Variant to the navigate function, or not?

Yeah in the end we need to pass a variant. We don't want to use string. This is just an experiment. This is not working yet. navigation is WIP.

Ok, would be great to add a comment there to know what's going there

In this case we don't. "NavigationProp" module is a binding to React Navigation's "navigation" that operates on strings and is not aware of ReasonML variant.

This is going to be super similar to

bs-react-navigation/src/StackNavigator.re

Lines 26 to 40 in aaf0f84

module NavigationProp = {

type t;

module State = {

type t;

[@bs.get] external getParams: t => option(routeProps) = "params";

};

[@bs.send] external push: (t, string, routeProps) => unit = "push";

[@bs.get "state"] external getState: t => State.t = "";

let getParams = t => getState(t) |> State.getParams;

};

and once we complete both, I think there's an opportunity to extract a common part.

We should translate variant to string here: https://github.com/callstackincubator/bs-react-navigation/pull/19/files#diff-f5a576ae9729bbd89c5aa8c4a2bf67ffR83

czystyl · 2018-12-31T06:49:08Z

src/TabNavigator.re

  let tabs =
    Config.order
    |> List.map(tab => {
-         let (tabname, screen, screenOptionsConfig) = Config.getTab(tab);
+         let (tabname, screen, screenOptionsConfig) =
+           Config.getTab(tab, makeNavigationProp());


makeNavigationProp in the future will accept original navigation prop. How do we handle that case? I know that the status of navigation prop is in WIP but I'm wondering how we handle this

I wonder that too 😉 We need to discuss that. You, me and @grabbou

I think we should handle that just like we handle it in StackNavigator ->

bs-react-navigation/src/StackNavigator.re

Lines 55 to 63 in aaf0f84

let makeNavigationProp = (navigation: NavigationProp.t) => {

push: route =>

NavigationProp.push(

navigation,

containerDisplayName,

routeProps(~route),

),

pop: _route => (),

};

src/TabNavigator.re

grabbou

Thank you @vnovick for working on it! I left few comments, in particular related to the navigation property to unblock you.

Looking forward to reviewing the final version!

grabbou · 2019-01-03T17:20:39Z

src/TabNavigator.re

  let tabBarOptions: tabBarOptions;
  let getTab:
-    tabs => (Js.Dict.key, navigation => ReasonReact.reactElement, screenOptions);
+    (tabs, navigation) =>
+    (Js.Dict.key, unit => ReasonReact.reactElement, screenOptions);


Does "screen" has to be a function?

unit => ReasonReact.reactElement

I would unify it with StackNavigator and just make it a React element. It will make the API much easier. If we need a functional form, we can always wrap it later on our side.

grabbou · 2019-01-03T17:24:48Z

src/TabNavigator.re

  let tabBarOptions: tabBarOptions;
  let getTab:
-    tabs => (Js.Dict.key, navigation => ReasonReact.reactElement, screenOptions);
+    (tabs, navigation) =>
+    (Js.Dict.key, unit => ReasonReact.reactElement, screenOptions);


We don't need users to define a string of a route explicitly. I know that TabNavigator's router requires us to use a concept of string to register a route.

We can do better.

There are two ways of handling that:

a) StackNavigator's way - we define only one "route", let's call it Container. Every "route" is a "" element that accepts current screen to render as a prop. For example, "" where "EditUser(5)" could be a ReasonML variant.

b) Define a hashing function that can generate a UUID based on a variant it is given. For example, for "EditUser(5)", this could be "Route_EditUser_5". It's representation is not important as long as the function is stable and can generate same string for a given variant. This will be only used internally by the React Navigation.

Given that TabNavigator doesn't have StackNavigator's limitations, I propose we go with "b" for the sake of simplicity. We can always change it later, transparently for the end users.

grabbou · 2019-01-03T17:27:33Z

src/TabNavigator.re

+    [@bs.send] external goBack: unit => unit = "goBack";
+  };
+
+  let makeNavigationProp = () => {


This should accept a "navigation" prop so that we can actually talk to a real "navigation". See how StackNavigation handles that. The code will be exactly the same, except for the "routeName" part. Depending on the approach you take, this will be either similar to StackNavigation (passing variant to a Container route) or generating a hash (to be used as a routeName) from a given variant.

Either way, "navigate" should accept a "tab" (variant) instead of a "routeName" (string). "NavigationProp.navigate" is good - ReactNavigation operates on "string"s and we should keep it that way.

grabbou · 2019-01-03T17:28:01Z

src/TabNavigator.re

  let tabs =
    Config.order
    |> List.map(tab => {
-         let (tabname, screen, screenOptionsConfig) = Config.getTab(tab);
+         let (tabname, screen, screenOptionsConfig) =


As already said, "tabName" could be generated dynamically. "screen" doesn't have to be a function, we can wrap it here if needed.

vnovick added 2 commits December 30, 2018 01:44

add Container support

82870c8

adding some tab navigator experiments + tab navigator nested support …

8b4b8f2

…experiment

vnovick requested review from grabbou and czystyl December 30, 2018 21:04

vnovick commented Dec 30, 2018

View reviewed changes

package.json Show resolved Hide resolved

refmt

aaf0f84

czystyl reviewed Dec 31, 2018

View reviewed changes

lewisf reviewed Jan 1, 2019

View reviewed changes

src/TabNavigator.re Show resolved Hide resolved

grabbou reviewed Jan 3, 2019

View reviewed changes

grabbou suggested changes Jan 3, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tab navigator experiments #19

Tab navigator experiments #19

vnovick commented Dec 30, 2018

czystyl Dec 31, 2018

vnovick Dec 31, 2018

czystyl Dec 31, 2018

grabbou Jan 3, 2019

grabbou Jan 3, 2019

czystyl Dec 31, 2018

vnovick Dec 31, 2018

grabbou Jan 3, 2019

grabbou left a comment

grabbou Jan 3, 2019

grabbou Jan 3, 2019

grabbou Jan 3, 2019

grabbou Jan 3, 2019

	module NavigationProp = {
	type t;

	module State = {
	type t;

	[@bs.get] external getParams: t => option(routeProps) = "params";
	};

	[@bs.send] external push: (t, string, routeProps) => unit = "push";

	[@bs.get "state"] external getState: t => State.t = "";

	let getParams = t => getState(t) \|> State.getParams;
	};

	let makeNavigationProp = (navigation: NavigationProp.t) => {
	push: route =>
	NavigationProp.push(
	navigation,
	containerDisplayName,
	routeProps(~route),
	),
	pop: _route => (),
	};

Tab navigator experiments #19

Are you sure you want to change the base?

Tab navigator experiments #19

Conversation

vnovick commented Dec 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grabbou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment