React router #343

Merged
bones7242 merged 96 commits from react-router into master 2018-02-15 08:02:17 +01:00
18 changed files with 150 additions and 221 deletions
Showing only changes of commit 4a2fbb402f - Show all commits

View file

@ -1,5 +1,7 @@
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
import * as actions from 'constants/show_action_types';
import { CHANNEL, ASSET_LITE, ASSET_DETAILS } from 'constants/show_request_types';
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
// basic request parsing
export function onRequestError (error) {
return {
@ -8,19 +10,22 @@ export function onRequestError (error) {
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 onParsedChannelRequest (name, id) {
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
const requestId = `cr#${name}#${id}`;
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 onNewChannelRequest (channelName, channelId) {
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
const requestType = CHANNEL;
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
const requestId = `cr#${channelName}#${channelId}`;
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.REQUEST_UPDATE_CHANNEL,
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
data: { requestId, name, id },
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
type: actions.CHANNEL_REQUEST_NEW,
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
data: { requestType, requestId, channelName, channelId },
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 onParsedAssetRequest (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
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
const requestType = extension ? ASSET_LITE : ASSET_DETAILS;
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
const requestId = `ar#${name}#${id}#${channelName}#${channelId}`;
return {
type: actions.REQUEST_UPDATE_ASSET,
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
type: actions.ASSET_REQUEST_NEW,
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
data: {
requestType,
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
requestId,
name,
modifier: {
@ -30,20 +35,12 @@ export function onParsedAssetRequest (name, id, channelName, channelId, extensio
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
id : 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
},
};
};
// asset actions
export function onNewAssetRequest (id, name, modifier) {
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 {
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
type: actions.ASSET_REQUEST_NEW,
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
data: { id, name, modifier },
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
};
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 addRequestToAssetRequests (id, error, name, claimId) {
return {
type: actions.ASSET_REQUEST_SUCCESS,
@ -60,13 +57,6 @@ export function addAssetToAssetList (id, error, name, claimId, shortId, claimDat
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
// channel actions
export function onNewChannelRequest (id, name, channelId) {
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 {
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
type: actions.CHANNEL_REQUEST_NEW,
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
data: {id, name, channelId},
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
};
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 addRequestToChannelRequests (id, error, name, longId, shortId) {
return {
type: actions.CHANNEL_REQUEST_ADD,

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

@ -1,32 +1,22 @@
import React from 'react';
import NavBar from 'containers/NavBar';
import AssetTitle from 'components/AssetTitle';
import AssetDisplay from 'components/AssetDisplay';
import AssetInfo from 'components/AssetInfo';
import { connect } from 'react-redux';
import View from './view';
class ShowAssetDetails extends React.Component {
render () {
return (
<div>
<NavBar/>
<div className="row row--tall row--padded">
<div className="column column--10">
<AssetTitle />
</div>
<div className="column column--5 column--sml-10 align-content-top">
<div className="row row--padded">
<AssetDisplay />
</div>
</div><div className="column column--5 column--sml-10 align-content-top">
<div className="row row--padded">
<AssetInfo />
</div>
</div>
</div>
}
</div>
);
}
const mapStateToProps = ({ show }) => {
console.log('mapping state to props', show);
// select request info
const requestId = show.request.id;
// select asset info
let asset;
const previousRequest = show.assetRequests[requestId] || null;
const assetList = show.assetList;
if (previousRequest) {
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`; // note: just store this in the request
asset = assetList[assetKey] || null;
};
// return props
return {
asset,
};
};
export default ShowAssetDetails;
export default connect(mapStateToProps, null)(View);

View file

@ -0,0 +1,39 @@
import React from 'react';
import NavBar from 'containers/NavBar';
import ErrorPage from 'components/ErrorPage';
import AssetTitle from 'components/AssetTitle';
import AssetDisplay from 'components/AssetDisplay';
import AssetInfo from 'components/AssetInfo';
class ShowAssetDetails extends React.Component {
render () {
const { asset } = this.props;
if (asset) {
return (
<div>
<NavBar/>
<div className="row row--tall row--padded">
<div className="column column--10">
<AssetTitle />
</div>
<div className="column column--5 column--sml-10 align-content-top">
<div className="row row--padded">
<AssetDisplay />
</div>
</div><div className="column column--5 column--sml-10 align-content-top">
<div className="row row--padded">
<AssetInfo />
</div>
</div>
</div>
}
</div>
);
};
return (
<ErrorPage error={'loading asset data...'}/>
);
}
};
export default ShowAssetDetails;

View file

@ -2,19 +2,21 @@ 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 }) => {
// select name and claim id
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}`
let name, claimId;
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 previousRequest = show.assetRequests[show.request.id];
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 assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`;
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 asset = show.assetList[assetKey];
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}`
if (asset) {
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}`
name = asset.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}`
claimId = asset.claimId;
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}`
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
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 requestId = show.request.id;
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 asset info
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}`
let asset;
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 previousRequest = show.assetRequests[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}`
console.log('previous request:', previousRequest);
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;
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}`
if (previousRequest) {
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 assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`; // 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}`
asset = assetList[assetKey] || 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}`
};
// return props
return {
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}`
claimId,
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}`
asset,
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}`
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

@ -4,13 +4,13 @@ import AssetDisplay from 'components/AssetDisplay';
class ShowLite extends React.Component {
render () {
const { name, claimId } = this.props;
const { asset } = this.props;
return (
<div className="row row--tall flex-container--column flex-container--center-center">
{ (name && claimId) &&
{ (asset) &&
<div>
<AssetDisplay />
<Link id="asset-boilerpate" className="link--primary fine-print" to={`/${claimId}/${name}`}>hosted via Spee.ch</Link>
<Link id="asset-boilerpate" className="link--primary fine-print" to={`/${asset.claimId}/${asset.name}`}>hosted via Spee.ch</Link>
</div>
}
</div>

View file

@ -0,0 +1,20 @@
import { connect } from 'react-redux';
import View from './view';
const mapStateToProps = ({ show }) => {
// select request info
const requestId = show.request.id;
// select request
const previousRequest = show.channelRequests[requestId] || null;
// select channel
let channel;
if (previousRequest) {
const channelKey = `c#${previousRequest.name}#${previousRequest.longId}`;
channel = show.channelList[channelKey] || null;
}
return {
channel,
};
};
export default connect(mapStateToProps, null)(View);

View file

@ -4,12 +4,12 @@ import NavBar from 'containers/NavBar';
import ChannelClaimsDisplay from 'containers/ChannelClaimsDisplay';
class ShowChannel extends React.Component {
componentDidMount () {
const { channel, requestId, requestChannelName, requestChannelId } = this.props;
if (!channel) {
return this.props.onNewChannelRequest(requestId, requestChannelName, requestChannelId);
}
}
// componentDidMount () {
// const { channel, requestId, requestChannelName, requestChannelId } = this.props;
// if (!channel) {
// return this.props.onNewChannelRequest(requestId, requestChannelName, requestChannelId); // check the channel you have in the request and see you have no channel so fetch that channel?
// }
// }
render () {
const { channel } = this.props;
if (channel) {

View file

@ -1,15 +1,13 @@
// request actions
export const REQUEST_UPDATE_CHANNEL = 'REQUEST_UPDATE_CHANNEL';
export const REQUEST_UPDATE_ASSET = 'REQUEST_UPDATE_ASSET';
export const REQUEST_UPDATE_ERROR = 'REQUEST_UPDATE_ERROR';
export const ASSET_REQUEST_NEW = 'ASSET_REQUEST_NEW';
export const CHANNEL_REQUEST_NEW = 'CHANNEL_REQUEST_NEW';
// asset actions
export const ASSET_REQUEST_NEW = 'ASSET_REQUEST_NEW';
export const ASSET_REQUEST_SUCCESS = 'ASSET_REQUEST_SUCCESS';
export const ASSET_ADD = `ASSET_ADD`;
// channel actions
export const CHANNEL_REQUEST_NEW = 'CHANNEL_REQUEST_NEW';
export const CHANNEL_REQUEST_ADD = 'CHANNEL_REQUEST_ADD';
export const CHANNEL_ADD = 'CHANNEL_ADD';

View file

@ -1,2 +1,3 @@
export const CHANNEL = 'CHANNEL';
export const ASSET = 'ASSET';
export const ASSET_LITE = 'ASSET_LITE';
export const ASSET_DETAILS = 'ASSET_DETAILS';

View file

@ -15,10 +15,8 @@ const mapStateToProps = ({ show }) => {
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 mapDispatchToProps = () => {
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
return {
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 mapDispatchToProps = {
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
onUpdateChannelClaims,
};
};
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
export default connect(mapStateToProps, mapDispatchToProps)(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

View file

@ -1,35 +0,0 @@
import { connect } from 'react-redux';
import View from './view';
import { onNewAssetRequest } from 'actions/show';
const mapStateToProps = ({ show }) => {
// select request info
const requestId = show.request.id;
const requestName = show.request.data.name;
const requestModifier = show.request.data.modifier;
const requestExtension = show.request.data.extension;
const assetList = show.assetList;
// select asset info
const previousRequest = show.assetRequests[show.request.id] || null;
let asset;
if (previousRequest) {
const assetKey = `a#${previousRequest.name}#${previousRequest.claimId}`; // note: just store this in the request
asset = assetList[assetKey] || null;
};
// return props
return {
requestId,
requestName,
requestModifier,
requestExtension,
asset,
};
};
const mapDispatchToProps = () => {
return {
onNewAssetRequest,
};
};
export default connect(mapStateToProps, mapDispatchToProps)(View);

View file

@ -1,24 +0,0 @@
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
import React from 'react';
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
import ErrorPage from 'components/ErrorPage';
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
import ShowAssetLite from 'components/ShowAssetLite';
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
import ShowAssetDetails from 'components/ShowAssetDetails';
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
class ShowAsset extends React.Component {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
componentDidMount () {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
const { asset, requestId, requestName, requestModifier } = this.props;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
if (!asset) {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
return this.props.onNewAssetRequest(requestId, requestName, requestModifier);
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
};
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
}
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
render () {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
const {asset, requestExtension} = this.props;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
if (asset) {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
return requestExtension ? <ShowAssetLite/> : <ShowAssetDetails/>;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
};
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
return (
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
<ErrorPage error={'loading asset data...'}/>
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
);
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
}
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
};
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
export default ShowAsset;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

View file

@ -1,32 +0,0 @@
import { connect } from 'react-redux';
import { onNewChannelRequest } from 'actions/show';
import View from './view';
const mapStateToProps = ({ show }) => {
// select request info
const requestId = show.request.id;
const requestChannelName = show.request.data.name;
const requestChannelId = show.request.data.id;
// select request
const previousRequest = show.channelRequests[show.request.id] || null;
// select channel
let channel;
if (previousRequest) {
const channelKey = `c#${previousRequest.name}#${previousRequest.longId}`;
channel = show.channelList[channelKey] || null;
}
return {
requestId,
requestChannelName,
requestChannelId,
channel,
};
};
const mapDispatchToProps = () => {
return {
onNewChannelRequest,
};
};
export default connect(mapStateToProps, mapDispatchToProps)(View);

View file

@ -1,5 +1,5 @@
import { connect } from 'react-redux';
import { onRequestError, onParsedChannelRequest, onParsedAssetRequest } from 'actions/show';
import { onRequestError, onNewChannelRequest, onNewAssetRequest } from 'actions/show';
import View from './view';
const mapStateToProps = ({ show }) => {
@ -9,12 +9,10 @@ const mapStateToProps = ({ show }) => {
};
};
const mapDispatchToProps = () => {
return {
const mapDispatchToProps = {
onRequestError,
onParsedChannelRequest,
onParsedAssetRequest,
};
onNewChannelRequest,
onNewAssetRequest,
};
export default connect(mapStateToProps, mapDispatchToProps)(View);

View file

@ -1,10 +1,11 @@
import React from 'react';
import ErrorPage from 'components/ErrorPage';
import ShowAsset from 'containers/ShowAsset';
import ShowChannel from 'containers/ShowChannel';
import ShowAssetLite from 'components/ShowAssetLite';
import ShowAssetDetails from 'components/ShowAssetDetails';
import ShowChannel from 'components/ShowChannel';
import lbryUri from 'utils/lbryUri';
import { CHANNEL, ASSET } from 'constants/show_request_types';
import { CHANNEL, ASSET_LITE, ASSET_DETAILS } from 'constants/show_request_types';
class ShowPage extends React.Component {
constructor (props) {
@ -42,9 +43,9 @@ class ShowPage extends React.Component {
}
// update the store
if (isChannel) {
return this.props.onParsedAssetRequest(claimName, null, channelName, channelClaimId, extension);
return this.props.onNewAssetRequest(claimName, null, channelName, channelClaimId, extension);
} else {
return this.props.onParsedAssetRequest(claimName, claimId, null, null, extension);
return this.props.onNewAssetRequest(claimName, claimId, null, null, extension);
}
}
parseAndUpdateClaimOnly (claim) {
@ -58,7 +59,7 @@ class ShowPage extends React.Component {
}
// return early if this request is for a channel
if (isChannel) {
return this.props.onParsedChannelRequest(channelName, channelClaimId);
return this.props.onNewChannelRequest(channelName, channelClaimId);
}
// if not for a channel, parse the claim request
let claimName, extension; // if I am destructuring below, do I still need to declare these here?
@ -67,7 +68,7 @@ class ShowPage extends React.Component {
} catch (error) {
return this.props.onRequestError(error.message);
}
this.props.onParsedAssetRequest(claimName, null, null, null, extension);
this.props.onNewAssetRequest(claimName, null, null, null, extension);
}
render () {
const { error, requestType } = this.props;
@ -79,8 +80,10 @@ class ShowPage extends React.Component {
switch (requestType) {
case CHANNEL:
return <ShowChannel />;
case ASSET:
return <ShowAsset />;
case ASSET_LITE:
return <ShowAssetLite />;
case ASSET_DETAILS:
return <ShowAssetDetails />;
default:
return <p>loading...</p>;
}

View file

@ -1,13 +1,11 @@
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
import * as actions from 'constants/show_action_types';
import { CHANNEL, ASSET } from 'constants/show_request_types';
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
import { LOCAL_CHECK, ERROR } from 'constants/asset_display_states';
const initialState = {
request: {
error: null,
type : null,
data : null,
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
requestId: null,
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
id : null,
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
},
channelRequests: {},
channelList : {},
@ -28,30 +26,13 @@ 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
error: action.data,
}),
});
case actions.REQUEST_UPDATE_CHANNEL:
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
case actions.CHANNEL_REQUEST_NEW:
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
case actions.ASSET_REQUEST_NEW:
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
return Object.assign({}, state, {
request: {
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 : CHANNEL,
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
error: null,
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
request: Object.assign({}, state.request, {
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: action.data.requestType,
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
id : action.data.requestId,
data : {
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
name: action.data.name,
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
id : action.data.id,
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
},
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
case actions.REQUEST_UPDATE_ASSET:
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
return Object.assign({}, state, {
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
request: {
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 : ASSET,
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
error: null,
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
id : action.data.requestId,
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
data : {
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
name : action.data.name,
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
modifier : action.data.modifier,
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
extension: action.data.extension,
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
},
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
});
// asset actions
case actions.ASSET_REQUEST_SUCCESS:

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

@ -4,7 +4,7 @@ import { addRequestToAssetRequests, onRequestError, addAssetToAssetList } from '
import { getLongClaimId, getShortId, getClaimData } from 'api/assetApi';
function* newAssetRequest (action) {
const { id, name, modifier } = action.data;
const { requestId, name, modifier } = action.data;
// get long id
console.log(`getting asset long id ${name}`);
let longId;
@ -15,7 +15,7 @@ function* newAssetRequest (action) {
return yield put(onRequestError(error.message));
}
// put action to add request to asset request list
yield put(addRequestToAssetRequests(id, null, name, longId));
yield put(addRequestToAssetRequests(requestId, null, name, longId));
// get short Id
console.log(`getting asset short id ${name} ${longId}`);
let shortId;

View file

@ -4,28 +4,28 @@ import { addNewChannelToChannelList, addRequestToChannelRequests, onRequestError
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 { getChannelClaims, getChannelData } from 'api/channelApi';
function* getNewChannelAndUpdateChannelList (action) {
const { id, name, channelId } = action.data;
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
const { requestId, channelName, channelId } = action.data;
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 long id
console.log('getting channel long id and short id');
let longId, shortId;
try {
({ data: {longChannelClaimId: longId, shortChannelClaimId: shortId} } = yield call(getChannelData, name, channelId));
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
({ data: {longChannelClaimId: longId, shortChannelClaimId: shortId} } = yield call(getChannelData, channelName, channelId));
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
} catch (error) {
return yield put(onRequestError(error.message));
}
// store the request in the channel requests list
yield put(addRequestToChannelRequests(id, null, name, longId, shortId));
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(addRequestToChannelRequests(requestId, null, channelName, longId, shortId));
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;
try {
({ data: claimsData } = yield call(getChannelClaims, name, longId, 1));
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
({ data: claimsData } = yield call(getChannelClaims, channelName, longId, 1));
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
} catch (error) {
return yield put(onRequestError(error.message));
}
// store the channel data in the channel list
const channelKey = `c#${name}#${longId}`;
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(addNewChannelToChannelList(channelKey, name, shortId, longId, 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
const channelKey = `c#${channelName}#${longId}`;
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(addNewChannelToChannelList(channelKey, channelName, shortId, longId, 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
// clear any request errors
yield put(onRequestError(null));
}

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