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

WIP Add Tracking #119

Merged
merged 20 commits into from Jun 21, 2018
Merged

WIP Add Tracking #119

merged 20 commits into from Jun 21, 2018

Conversation

benshope
Copy link
Contributor

@benshope benshope commented Jun 15, 2018

This adds some Google Analytics tracking to the demo site.

Resolves #116

@@ -3,6 +3,15 @@
<head>
<meta charset='UTF-8' />
<title>kepler.gl demo</title>
<script async src="https://www.googletagmanager.com/gtag/js?id=UA-64694404-15"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to add it here. it is already in website/index.html

@@ -0,0 +1,53 @@
import { ActionTypes } from "kepler.gl/actions";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please confit your editor settings to get rid of extra space in the bracket

Copy link
Contributor Author

@benshope benshope Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the .prettierrc to the root directory fixes this - if it's okay to do that

import { LOCATION_CHANGE } from "react-router-redux";

const {
ADD_DATA,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe easier to not de-construct here and just use ActionTypes. ADD_DATA in the following function. So that people don't have to edit in 2 places when adding new action to be tracked

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, yep!


const trackingInformation = {
[ADD_DATA]: ({ payload: { files } }) =>
files.map(({ name, size, type }) => ({ name, size, type })),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not track the name, only type and size would be sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, better not to upload anything about the users' data that they might not want

const getPayload = ({ payload }) => payload;

const trackingInformation = {
[ADD_DATA]: ({ payload: { files } }) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ADD_DATA is not used, LOAD_FILES is the one you want to track

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

@benshope

This comment has been minimized.

@@ -0,0 +1,78 @@
import {ActionTypes} from 'kepler.gl/actions';

This comment was marked as resolved.

This comment was marked as resolved.

@@ -27,13 +27,15 @@ import window from 'global/window';
import {taskMiddleware} from 'react-palm';

import demoReducer from './reducers';
import analyticsMiddleware from "./analytics";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be single quotes?

@chrisirhc

This comment has been minimized.

@benshope

This comment has been minimized.

info: {lngLat}
}
}) => lngLat,
[ActionTypes.LAYER_CONFIG_CHANGE]: ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LAYER_TYPE_CHANGE

[ActionTypes.MAP_STYLE_CHANGE]: getPayload,
[ActionTypes.TOGGLE_MODAL]: getPayload,
[ActionTypes.TOGGLE_SIDE_PANEL]: getPayload,
[ActionTypes.UPDATE_MAP]: withStoreInformation(getPayload),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is map viewport change, don't track

}),
[ActionTypes.MAP_STYLE_CHANGE]: getPayload,
[ActionTypes.TOGGLE_MODAL]: getPayload,
[ActionTypes.TOGGLE_SIDE_PANEL]: getPayload,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to track this

[LOCATION_CHANGE]: x => x
};

const analyticsMiddleware = store => next => action => {

This comment was marked as resolved.

This comment was marked as resolved.

[ActionTypes.TOGGLE_MODAL]: getPayload,
[ActionTypes.TOGGLE_SIDE_PANEL]: getPayload,
[ActionTypes.UPDATE_MAP]: withStoreInformation(getPayload),
[ActionTypes.SET_FILTER]: withStoreInformation(getPayload),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume this is where we track the type of filter being used. payload would be not wrapped. and it would sed {idx, prop, value}

const {prop} = payload;
if (prop === 'name') {
// get the type of the filter from the next store
const visState = store.demo.keplerGl.map.visState;
const filter  = visState.filters[idx]
const newType = filter.type;

const trackingPayload = {type: newType}
}

const trackingInformation = {
[ActionTypes.LOAD_FILES]: ({files}) =>
files.map(({size, type}) => ({size, type})),
[ActionTypes.LAYER_HOVER]: ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to track LAYER_HOVER, it gets called all the time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All actions are currently tracked - should I specifically exclude this one?

[ActionTypes.MAP_STYLE_CHANGE]: getPayload,
[ActionTypes.TOGGLE_MODAL]: getPayload,
[ActionTypes.SET_FILTER]: getFilterFromStore,
[ActionTypes.INTERACTION_CONFIG_CHANGE]: ({config}) => config,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we want to track whether tooltip or brushing is enabled, we should just track that
({config}) => ({[config.id]:{config.enabled]})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

[ActionTypes.TOGGLE_MODAL]: getPayload,
[ActionTypes.SET_FILTER]: getFilterFromStore,
[ActionTypes.INTERACTION_CONFIG_CHANGE]: ({config}) => config,
[LOCATION_CHANGE]: x => x
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to track how many layers and filters we can do

[ActionTypes.ADD_LAYER]: (payload, store) => ({
  total: store.demo.keplerGl.map.visState.layers.length
}),
[ActionTypes.ADD_FILTER]: (payload, store) => ({
  total: store.demo.keplerGl.map.visState.filters.length
}),

@@ -273,6 +277,14 @@ function PanelHeaderFactory() {
);
}
}
return connect(undefined, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are adding tracking to kepler.gl/src directly here. We don't want to do that. Also, we should now randomly call connect in just this component. connect should only called once in the top level component. If you need, we can add a new ui-state action TOGGLE_SIDE_PANEL_HEADER and showHeaderDropdown prop in ui-state to trigger this behavior

@benshope
Copy link
Contributor Author

benshope commented Jun 20, 2018

@heshan0131 this should be good to deploy - I had to fix a few very strange test failures - so give it a thorough last check (particularly those parts)

<StyledPanelHeader className="side-panel__panel-header">
<StyledPanelHeaderTop className="side-panel__panel-header__top">
<logoComponent version={version}/>
<this.props.logoComponent appName={appName} version={version}/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PanelHeader can't be rendered as a pure functional component, because we can only use this.props.logoComponent syntax to render the local component, otherwise, react will interpret logoComponent as a tag

@heshan0131 heshan0131 merged commit 0822a5e into master Jun 21, 2018
@macrigiuseppe macrigiuseppe deleted the tracking branch October 21, 2018 23:48
bjungs pushed a commit to imec-int/kepler.gl that referenced this pull request Nov 14, 2022
…etail-page

[BUBR-908] Mobilize detail page
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.

None yet

3 participants