React router #343

Merged
bones7242 merged 96 commits from react-router into master 2018-02-15 08:02:17 +01:00
11 changed files with 14 additions and 16 deletions
Showing only changes of commit 0548906c4d - Show all commits

View file

@ -39,7 +39,7 @@ export function onNewAssetRequest (name, id, channelName, channelId, extension)
neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async
neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async
};
};
export function addRequestToPreviousRequests (id, error, key) {
neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async
export function addRequestToRequestList (id, error, key) {
neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async
return {
type: actions.PREVIOUS_REQUEST_ADD,
data: { id, error, key },

neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async
neb-b commented 2018-02-05 20:57:15 +01:00 (Migrated from github.com)
Review

I think generally the pattern is that an action is { type: "some string", data: { name, id... } } just to keep things consistent. data can be an object or a string, but I think it's helpful to put everything inside of that

I think generally the pattern is that an action is `{ type: "some string", data: { name, id... } }` just to keep things consistent. `data` can be an object or a string, but I think it's helpful to put everything inside of that
neb-b commented 2018-02-13 06:05:56 +01:00 (Migrated from github.com)
Review

This probably shouldn't be called XXX_ASYNC since it isn't async

This probably shouldn't be called `XXX_ASYNC` since it isn't async

View file

@ -7,7 +7,7 @@ const mapStateToProps = ({ show }) => {
neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
const error = show.displayAsset.error;
const status = show.displayAsset.status;
// select asset
const request = show.previousRequests[show.request.id];
neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
const request = show.requestList[show.request.id];
neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
const assetKey = request.key;
const asset = show.assetList[assetKey];
// return props

neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.
neb-b commented 2018-02-05 20:47:37 +01:00 (Migrated from github.com)
Review

Why do you do const that = this?

Why do you do `const that = this`?
neb-b commented 2018-02-05 20:52:52 +01:00 (Migrated from github.com)
Review

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same <AssetDisplay /> it will make these requests again, even if you just made them a second ago

I think this is another piece you can move entirely into redux. Currently if this component is rendered, then a user navigates away and comes back to the same `<AssetDisplay />` it will make these requests again, even if you just made them a second ago
bones7242 commented 2018-02-07 00:13:24 +01:00 (Migrated from github.com)
Review

I had a misunderstanding of how the this context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

I had a misunderstanding of how the `this` context works and when I needed to pass this in to a function manually. I was able to remove it from the app in multiple places where it isn't necessary.

View file

@ -3,7 +3,7 @@ import View from './view';
neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.
neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.
const mapStateToProps = ({ show }) => {
// select asset
const request = show.previousRequests[show.request.id];
neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.
const request = show.requestList[show.request.id];
neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.
const assetKey = request.key;
const asset = show.assetList[assetKey];
// return props

neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.
neb-b commented 2018-02-05 20:39:44 +01:00 (Migrated from github.com)
Review

This should be a button if it isn't linking anywhere.

This should be a `button` if it isn't linking anywhere.

View file

@ -3,7 +3,7 @@ import View from './view';
const mapStateToProps = ({ show }) => {
// select title
const request = show.previousRequests[show.request.id];
const request = show.requestList[show.request.id];
const assetKey = request.key;
const asset = show.assetList[assetKey];
let title;

View file

@ -2,12 +2,11 @@ import { connect } from 'react-redux';
import View from './view';
const mapStateToProps = ({ show }) => {
console.log('mapping state to props', show);
// select request info
const requestId = show.request.id;
// select asset info
let asset;
const request = show.previousRequests[requestId] || null;
const request = show.requestList[requestId] || null;
const assetList = show.assetList;
if (request && assetList) {
const assetKey = request.key; // note: just store this in the request

View file

@ -2,12 +2,11 @@ import { connect } from 'react-redux';
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
import View from './view';
const mapStateToProps = ({ show }) => {
console.log('mapping state to props', show);
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
// select request info
const requestId = show.request.id;
// select asset info
let asset;
const request = show.previousRequests[requestId] || null;
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
const request = show.requestList[requestId] || null;
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
const assetList = show.assetList;
if (request && assetList) {
const assetKey = request.key; // note: just store this in the request

neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

You can use destructuring twice to avoid all the repeated this.props.claimData

const { claimData: { name, claimId... } } = this.props

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`

View file

@ -5,7 +5,7 @@ const mapStateToProps = ({ show }) => {
// select request info
const requestId = show.request.id;
// select request
const previousRequest = show.previousRequests[requestId] || null;
const previousRequest = show.requestList[requestId] || null;
// select channel
let channel;
if (previousRequest) {

View file

@ -4,7 +4,7 @@ import View from './view';
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
const mapStateToProps = ({ show }) => {
// select channel key
const request = show.previousRequests[show.request.id];
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
const request = show.requestList[show.request.id];
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
const channelKey = request.key;
// select channel claims
const channel = show.channelList[channelKey] || null;

neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

Will these nested values always exist?
bones7242 commented 2018-02-07 07:58:43 +01:00 (Migrated from github.com)
Review

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?

They will always exist when ChannelClaimsDisplay is rendered, unless that should change... I am updated it to be destructured, is that what you were thinking?
neb-b commented 2018-02-07 08:10:06 +01:00 (Migrated from github.com)
Review

I was just wondering if there would ever be a case when show is undefined. Which would cause an errror. cannot read property 'showChannel of undefined`.

Or if any of those children would be undefined which would throw an error

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error

View file

@ -7,7 +7,7 @@ const initialState = {
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
type : null,
id : null,
},
previousRequests: {},
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
requestList: {},
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
channelList : {},
assetList : {},
displayAsset : {
@ -36,7 +36,7 @@ export default function (state = initialState, action) {
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
// store requests
case actions.PREVIOUS_REQUEST_ADD:
return Object.assign({}, state, {
previousRequests: Object.assign({}, state.previousRequests, {
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
requestList: Object.assign({}, state.requestList, {
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
[action.data.id]: {
error: action.data.error,
key : action.data.key,

neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
neb-b commented 2018-02-05 20:12:41 +01:00 (Migrated from github.com)
Review

In the app we use a util to avoid a lot of the boiler plate with redux.
https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js

It just makes it so you don't need to use a switch. I really like it.

In the app we use a util to avoid a lot of the boiler plate with redux. https://github.com/lbryio/lbry-app/blob/master/src/renderer/util/redux-utils.js It just makes it so you don't need to use a switch. I really like it.
bones7242 commented 2018-02-09 20:29:01 +01:00 (Migrated from github.com)
Review

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99

View file

@ -1,6 +1,6 @@
import { call, put, takeLatest } from 'redux-saga/effects';
import * as actions from 'constants/show_action_types';
import { addRequestToPreviousRequests, onRequestError, addAssetToAssetList } from 'actions/show';
import { addRequestToRequestList, onRequestError, addAssetToAssetList } from 'actions/show';
import { getLongClaimId, getShortId, getClaimData } from 'api/assetApi';
function* newAssetRequest (action) {
@ -16,7 +16,7 @@ function* newAssetRequest (action) {
}
// put action to add request to asset request list
const assetKey = `a#${name}#${longId}`;
yield put(addRequestToPreviousRequests(requestId, null, assetKey));
yield put(addRequestToRequestList(requestId, null, assetKey));
// get short Id
console.log(`getting asset short id ${name} ${longId}`);
let shortId;

View file

@ -1,6 +1,6 @@
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
import { call, put, takeLatest } from 'redux-saga/effects';
import * as actions from 'constants/show_action_types';
import { addNewChannelToChannelList, addRequestToPreviousRequests, onRequestError, updateChannelClaims } from 'actions/show';
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
import { addNewChannelToChannelList, addRequestToRequestList, onRequestError, updateChannelClaims } from 'actions/show';
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
import { getChannelClaims, getChannelData } from 'api/channelApi';
function* getNewChannelAndUpdateChannelList (action) {
@ -15,7 +15,7 @@ function* getNewChannelAndUpdateChannelList (action) {
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
}
// store the request in the channel requests list
const channelKey = `c#${channelName}#${longId}`;
yield put(addRequestToPreviousRequests(requestId, null, channelKey));
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
yield put(addRequestToRequestList(requestId, null, channelKey));
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
// get channel claims data
console.log('getting channel claims data');
let claimsData;

neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

I don't have any experience using redux-saga so I might just not understand what is happening.

Generally you want XX_SUCCESS and XX_FAIL actions separated. That makes it a lot easier handling the data on the reducer.

I don't have any experience using `redux-saga` so I might just not understand what is happening. Generally you want `XX_SUCCESS` and `XX_FAIL` actions separated. That makes it a lot easier handling the data on the reducer.
neb-b commented 2018-02-09 22:55:57 +01:00 (Migrated from github.com)
Review

If these aren't being used anywhere else, I don't think they need to be separated.

IMO it would make these saga files easier to understand/follow, but not a big issue

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue