WIP: Add redux #1130

crcommons · 2018-05-04T20:37:59Z

This pull request makes the following changes:

Adds Redux, Routing, and fake components to test

Testing checklist:

Go to home page and confirm you see in the mode bar: "junk", "Foo: 3", and "Baz: 2"
Go to "settings/testroute/1, click the "Click Me" link. You should be taken to data view
I certify that I ran my checklist

Fixes ushahidi/platform#2907
Fixes ushahidi/platform#2826

Ping @ushahidi/platform

…gure out connect

…njected

rjmackay · 2018-05-06T23:34:52Z

A few thoughts as I read this code, and various docs and issues

We can probably inject a store into react components like this: Add Redux store to render. coatue-oss/react2angular#58 (comment)
However ngRedux defaults to creating the store itself, so we need to either
a. Pull the store out of ngRedux to provide it to react. I think thats just $ngRedux itself.
b. Create a store the usual react way and inject to angular. It looks like you can now do this: Feature Request: use an already created store. angular-redux/ng-redux#19

…tate Sadly the syntax isn't a perfect match for connect: `connectWithStore(MyComponent, mapStateToProps, mapDispatchToProps)` not `connect(mapStateToProps, mapDispatchToProps)(MyComponent)`

- Export RootReducer as default - Import RootReducer rather than require - Replace Form with forms in root reducer - Add some junk default state - Output junk default state in ush logo component

rjmackay · 2018-05-07T00:56:31Z

@crcommons I pushed a couple of commits they

make redux store connect to the component using the method I found here Add Redux store to render. coatue-oss/react2angular#58 (comment)
fix some weirdness in the state shape caused by the way you were using require/export
Basically the state looked like this...

{
  RootReducer : {
    Forms: {
      forms: []
    }
  }
}

I figured you probably didn't want 'RootReducer' at the top? and renamed Forms to forms

rjmackay · 2018-05-07T00:57:33Z

app/react-transition/connectWithStore.jsx

@@ -0,0 +1,18 @@
+import React from 'react';


I wasn't sure where to put this, so I started a react-transition folder. Maybe it should just be in common/ or common/react-transition

rjmackay · 2018-05-07T00:57:41Z

app/react-transition/connectWithStore.jsx

+  return (props) => {
+    // Get the injector + store at render time
+    // Otherwise it happens before the angular app is set up
+    const injector = angular.element(document).injector();


This is pretty gross but it works :)

rjmackay · 2018-05-07T00:58:59Z

app/common/directives/ush-logo-react/ush-logo.jsx


 MyComponent.propTypes = {
  foo: PropTypes.number.isRequired,
  baz: PropTypes.number.isRequired
 };
+
+export default connectWithStore(MyComponent, mapStateToProps, mapDispatchToProps);


I couldn't figure out if it was possible to make this work with connect style syntax ie.
connect(mapStateToProps, mapDispatchToProps)(MyComponent)
so for now it has the component as the first prop instead.

rjmackay · 2018-05-07T01:01:16Z

app/common/directives/ush-logo-react/ush-logo.reducer.js

+    }
+}
+
+export const getFormsFromState = state => state.forms.forms;


Kind of a tangent: but I always find it weird that the ~~mappers~~ selectors for a particular reducer have to know what namespace this reducer is used under. ie. if you change the name of forms in rootReducer, this code breaks. I don't know if theres a standard solution for that at all, ie only passing the forms part of state to this function?

I've done some research on this and am really not sure. Adding it as an issue to refactor later.

…t props

rjmackay · 2018-05-08T22:39:58Z

app/main/posts/views/mode-context.html

@@ -17,5 +17,5 @@ <h1 class="mode-context-title"><a href="/" ng-bind="nav.site.name"></a></h1>
        <!-- <saved-search-create></saved-search-create> -->
        <mode-context-form-filter></mode-context-form-filter>
    </div>
-    <my-component foo=3 baz=2></my-component>
+    <my-component store={store} foo=3 baz=2></my-component>


I missed this - but we shouldn't need to pass in store here. It might break things

rjmackay · 2018-05-08T22:40:11Z

app/common/directives/mode-bar/mode-bar.directive.js

@@ -38,6 +40,7 @@ function (
            $scope.logout = Authentication.logout;
            $scope.register = Registration.openRegister;
            $scope.intercomAppId = $window.ushahidi.intercomAppId;
+            $scope.store = $ngRedux;


Need to remove this.

…and uch

feat: Add routing using a wrapper controller

…r in the future

Made it so that Jest isn't mad at angular and now tests are passing

Checking for store and reassigning props broke connect component; removed and just using plain component instead

test(ush-logo test files): got tests working with store data

…ngular/jest incompatibility Link.test.jsx

event is never received in tests

Mock state and validate calls to state.go

Migration/link api

rjmackay · 2018-05-15T00:04:45Z

.gitignore

@@ -23,3 +23,4 @@ server/www/img/icons/
 server/www/
 app/locales/
 package-lock.json
+app.stats.html


Shouldn't that be app/stats.html ?

rjmackay · 2018-05-15T00:05:21Z

app/app.js

+require('ng-redux');
+const thunk = require('redux-thunk');
+import RootReducer from './rootReducer';
+// import { combineReducers } from 'redux';


Clean up the comments?

rjmackay · 2018-05-15T00:05:33Z

app/app.js

@@ -18,6 +18,15 @@ require('angular-nvd3/src/angular-nvd3');
 require('angular-cache');
 require('angular-linkify');

+require('redux');
+require('ng-redux');
+const thunk = require('redux-thunk');


We're not using thunk yet right?

rjmackay · 2018-05-15T00:05:59Z

app/app.js

@@ -18,6 +18,15 @@ require('angular-nvd3/src/angular-nvd3');
 require('angular-cache');
 require('angular-linkify');

+require('redux');


Do we need to require redux? or just ng-redux?

rjmackay

Some minor clean up requested in comments.
Just clean those up and this can land

…er because app.js just simply doesn't work with new linting.

crcommons added 3 commits May 4, 2018 14:37

skeleton on redux together, but its not working yet.

4e3f154

apparently import breaks everything. using require instead. now to fi…

52f9c83

…gure out connect

redux works when I remove thunk middlewear, but now can't get store i…

9e9a4f2

…njected

rjmackay added 2 commits May 7, 2018 12:51

fix: Add custom connectFromStore() to connect components to ngRedux s…

c693962

…tate Sadly the syntax isn't a perfect match for connect: `connectWithStore(MyComponent, mapStateToProps, mapDispatchToProps)` not `connect(mapStateToProps, mapDispatchToProps)(MyComponent)`

fix: Fix up ngRedux state setup to avoid 'RootReducer' wrapper

9db1db1

- Export RootReducer as default - Import RootReducer rather than require - Replace Form with forms in root reducer - Add some junk default state - Output junk default state in ush logo component

rjmackay reviewed May 7, 2018

View reviewed changes

fix: Copy propTypes to connected component so react2angular can detec…

db8ea73

…t props

rjmackay reviewed May 8, 2018

View reviewed changes

rjmackay and others added 10 commits May 9, 2018 11:53

Remove unneeded manual store binding

fc5c6c3

feat: Add routing using a wrapper controller

5323950

style(New react components and settings.module): cleaning up spacing …

d9044a1

…and uch

Merge pull request #1134 from ushahidi/react-routing-wrapper

0a447fa

feat: Add routing using a wrapper controller

created Link API which will allow us to easily switch out react-route…

6ebf554

…r in the future

style updates

cb3a79e

test(ush-logo test files): got tests working with store data

96f5e61

Made it so that Jest isn't mad at angular and now tests are passing

fix(connect, mycomponent): removed check and reassign of props

92ac2e8

Checking for store and reassigning props broke connect component; removed and just using plain component instead

ci: Update travis to run node 6+

9dbc129

Merge pull request #1138 from ushahidi/redux-fix-tests

41e5a4b

test(ush-logo test files): got tests working with store data

ushbot temporarily deployed to rackspace_pr May 10, 2018 16:12 Inactive

crcommons and others added 4 commits May 10, 2018 10:14

Merge branch 'add-redux' into migration/Link-API

ffdebd0

feat(Routing): Link router is complete; tests don't work because of a…

2f89854

…ngular/jest incompatibility Link.test.jsx

test: bootstrap angular before Link tests

7cc6522

fix: Don't try to stop Link event propagation

953d26b

event is never received in tests

rjmackay and others added 3 commits May 11, 2018 16:23

text: Check Link component calls state.go

4cd6d09

Mock state and validate calls to state.go

test(Link snapshot): pushing snapshot for travis tests

3284850

Merge pull request #1139 from ushahidi/migration/Link-API

a000bdb

Migration/link api

ushbot temporarily deployed to rackspace_pr May 11, 2018 16:39 Inactive

resolving merge conflicts

7f7eaa6

ushbot temporarily deployed to rackspace_pr May 11, 2018 17:41 Inactive

chore(Prettier config): changed spacing from 2 to 4

68babc1

ushbot temporarily deployed to rackspace_pr May 11, 2018 18:13 Inactive

ushahidi deleted a comment from rjmackay May 11, 2018

rjmackay reviewed May 15, 2018

View reviewed changes

rjmackay approved these changes May 15, 2018

View reviewed changes

adding thunk middlewear correctly and fixing gitignore; ignoring lint…

ab1bd75

…er because app.js just simply doesn't work with new linting.

ushbot temporarily deployed to rackspace_pr May 15, 2018 18:54 Inactive

crcommons merged commit 3a92e18 into experiment/react-angular May 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add redux #1130

WIP: Add redux #1130

crcommons commented May 4, 2018 •

edited

Loading

rjmackay commented May 6, 2018

rjmackay commented May 7, 2018

rjmackay May 7, 2018

rjmackay May 7, 2018

rjmackay May 7, 2018

rjmackay May 7, 2018 •

edited

Loading

crcommons May 11, 2018

rjmackay May 8, 2018

rjmackay May 8, 2018

rjmackay May 15, 2018

rjmackay May 15, 2018

rjmackay May 15, 2018

rjmackay May 15, 2018

rjmackay left a comment

WIP: Add redux #1130

WIP: Add redux #1130

Conversation

crcommons commented May 4, 2018 • edited Loading

rjmackay commented May 6, 2018

rjmackay commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjmackay May 7, 2018 • edited Loading

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

rjmackay left a comment

Choose a reason for hiding this comment

crcommons commented May 4, 2018 •

edited

Loading

rjmackay May 7, 2018 •

edited

Loading