react-navigation / redux-helpers

Redux middleware and utils for React Navigation
Other
296 stars 43 forks source link

Add screen tracker middleware #6

Closed yordis closed 6 years ago

yordis commented 6 years ago
import { GoogleAnalyticsTracker } from "react-native-google-analytics-bridge";
import { createScreenTrackingMiddleware } from "react-navigation-redux-helpers";

const  navStateSelector = (state) => state.navigation

const ScreenTrackingMiddleware = createScreenTrackingMiddleware(
  navStateSelector,
  (fromScreen, toScreen) => {
    tracker.trackScreenView(toScreen);
  }
);
yordis commented 6 years ago

@brentvatne @Ashoat ready to merge,

any feedback?

yordis commented 6 years ago

@Ashoat if you like it do not merge so I go and update the documentation and leave a PR open so I dont forget about it 😄

yordis commented 6 years ago

@Ashoat do you think is possible to get this into the package this week? I want to create another PR with a HOC that will speed up the development as well.

Ashoat commented 6 years ago

Hey @yordis, sorry for the delay in reviewing here. I don't think it makes sense to include this screen-tracking middleware in this package. I want to keep the scope of this package small - basically, just the pieces necessary to implement basic Redux integration - so that it's easy for package users to grok, and to keep the cost of maintenance low.

Sorry and I hope you understand! I think this would be great as a separate package.

yordis commented 6 years ago

@Ashoat I can accept it but I can understand.

Just think how many people you would help and issues you would avoid by adding such of packages.

Advance users could always add their own workflow.

I want to keep the scope of this package small - basically, just the pieces necessary to implement basic Redux integration

If that is the case, why did you took the PR for the reducer createNavigationReducer? Don't get cut off on what you want but what is beneficial for others ( I really dont mind about this because I can just create my own package and move on, I dont need this, but I am thinking on how many people I could help).

Anyway,

I hope you change your mind on this, a lot of people will appreciated adding less friction and for more Seniors I wanted to update the documentation with both cases.

crobinson42 commented 6 years ago

@yordis curious if you published a package to capture the screens in redux transitions for react-navigation?

Thanks!

yordis commented 6 years ago

@crobinson42 Hey here you go https://github.com/straw-hat-team/react-navigation-redux-helpers

Since RN and React-Navigation introduce breaking changes sometimes, I am not sure if it will work.

In any case, please use it and open an issue if anything breaks. I fix it right away.

yordis commented 6 years ago
import { createScreenTrackingMiddleware } from "@straw-hat/react-navigation-redux-helpers";

const onScreenChange = (currentScreen: any, nextScreen: any) => {
  Analytics.trackEvent('user_navigation', {currentScreen, nextScreen})
};

export const navStateSelector = (state: any) => state.navigation;

const screenTrackingMiddleware = createScreenTrackingMiddleware(
  navStateSelector,
  onScreenChange
);

// Add screenTrackingMiddleware to your reducer middlewares