React router #343

Merged
bones7242 merged 96 commits from react-router into master 2018-02-15 08:02:17 +01:00
9 changed files with 48 additions and 56 deletions
Showing only changes of commit c4042ecea9 - Show all commits

View file

@ -107,10 +107,10 @@ export function showNewChannel (id, channelData) {
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 updateShowChannel (error, name, shortId, longId, claimData) {
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 updateShowChannel (error, name, shortId, longId, claimsData) {
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.SHOW_CHANNEL_UPDATE,
data: { error, name, shortId, longId, claimData },
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: { error, name, shortId, longId, claimsData },
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

View file

@ -1,6 +1,7 @@
import Request from 'utils/request';
export function getLongClaimId (name, modifier) {
console.log('getting long claim id for asset:', name, modifier);
let body = {};
// create request params
if (modifier) {
@ -26,11 +27,13 @@ export function getLongClaimId (name, modifier) {
};
export function getShortId (name, claimId) {
console.log('getting short id for asset:', name, claimId);
const url = `/api/claim/short-id/${claimId}/${name}`;
return Request(url);
};
export function getClaimData (name, claimId) {
console.log('getting claim data for asset:', name, claimId);
const url = `/api/claim/data/${name}/${claimId}`;
return Request(url);
};

View file

@ -2,26 +2,27 @@ import { connect } from 'react-redux';
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
import { updateChannelClaimsData } from 'actions/show';
import View from './view';
const mapStateToProps = ({ show : { showChannel: { channelData, channelClaimsData } } }) => {
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

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

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

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

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

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

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
const mapStateToProps = ({ show : { showChannel: { error, channelData, claimsData } } }) => {
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 {
error : 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
name : channelData.name,
longId : channelData.longId,
claims : channelClaimsData.claims,
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
currentPage: channelClaimsData.currentPage,
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
totalPages : channelClaimsData.totalPages,
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
totalClaims: channelClaimsData.totalClaims,
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
claims : claimsData.claims,
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
currentPage: claimsData.currentPage,
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
totalPages : claimsData.totalPages,
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
totalClaims: claimsData.totalClaims,
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 = dispatch => {
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
onChannelClaimsDataUpdate: (claims, currentPage, totalPages, totalClaims) => {
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
dispatch(updateChannelClaimsData(claims, currentPage, totalPages, totalClaims));
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
onChannelClaimsDataClear: () => {
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
dispatch(updateChannelClaimsData(null, null, null, null));
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

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

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

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

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

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

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

Will these nested values always exist?

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

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

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

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

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

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
};
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 = dispatch => {
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
// onChannelClaimsDataUpdate: (claims, currentPage, totalPages, totalClaims) => {
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
// dispatch(updateChannelClaimsData(claims, currentPage, totalPages, totalClaims));
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
// onChannelClaimsDataClear: () => {
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
// dispatch(updateChannelClaimsData(null, null, null, null));
neb-b commented 2018-02-05 20:27:54 +01:00 (Migrated from github.com)
Review

Will these nested values always exist?

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

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

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

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

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

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

Will these nested values always exist?

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

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

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

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

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

I was just wondering if there would ever be a case when `show` is undefined. Which would cause an errror. `cannot read property 'showChannel` of undefined`. Or if any of those children would be undefined which would throw an error
// };
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
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
export default connect(mapStateToProps, null)(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
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,54 +1,38 @@
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
import React from 'react';
import AssetPreview from 'components/AssetPreview';
import request from 'utils/request';
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
class ChannelClaimsDisplay extends React.Component {
constructor (props) {
super(props);
this.state = {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
error: null,
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
};
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.showNextResultsPage = this.showNextResultsPage.bind(this);
this.showPreviousResultsPage = this.showPreviousResultsPage.bind(this);
}
componentDidMount () {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
const name = this.props.name;
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
const longId = this.props.longId;
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.updateClaimsData(name, longId, 1);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
componentWillReceiveProps (nextProps) {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
if (nextProps.name !== this.props.name || nextProps.longId !== this.props.longId) {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.updateClaimsData(nextProps.name, nextProps.longId, 1);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
updateClaimsData (name, longId, page) {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
console.log('this function has been moved into the redux sagas');
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
componentWillUnmount () {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.props.onChannelClaimsDataClear();
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
showNewPage (page) {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
console.log(`update claims data with new page ${page}`);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
showPreviousResultsPage () {
const previousPage = parseInt(this.props.currentPage) - 1;
this.updateClaimsData(this.props.name, this.props.longId, previousPage);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.showNewPage(previousPage);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
showNextResultsPage () {
const nextPage = parseInt(this.props.currentPage) + 1;
this.updateClaimsData(this.props.name, this.props.longId, nextPage);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
this.showNewPage(nextPage);
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
}
render () {
const { error, claims, currentPage, totalPages } = this.props;
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
return (
<div>
{this.state.error ? (
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
{error ? (
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
<div className="row">
<div className="column column--10">
<p>{this.state.error}</p>
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
<p>{error}</p>
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
</div>
</div>
) : (
<div className="row row--tall">
{this.props.claims &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
{claims &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
<div>
{this.props.claims.map((claim, index) => <AssetPreview
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
{claims.map((claim, index) => <AssetPreview
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
name={claim.name}
claimId={claim.claimId}
fileExt={claim.fileExt}
@ -56,10 +40,10 @@ class ChannelClaimsDisplay extends React.Component {
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
key={`${claim.name}-${index}`}
/>)}
<div>
{(this.props.currentPage > 1) &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
{(currentPage > 1) &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
<button className={'button--secondary'} onClick={this.showPreviousResultsPage}>Previous Page</button>
}
{(this.props.currentPage < this.props.totalPages) &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
{(currentPage < totalPages) &&
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
<button className={'button--secondary'} onClick={this.showNextResultsPage}>Next Page</button>
}
</div>

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

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)
neb-b commented 2018-02-05 20:26:45 +01:00 (Migrated from github.com)
Review

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch updateClaimsData action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

This might be what you were thinking of doing, but instead of doing the request here, then calling an action to update the data or set an error, just dispatch `updateClaimsData` action which makes the call, then updates the redux state accordingly. I think making an effort to keep all data logic inside of redux files can simplify a lot of components (for the most part)

View file

@ -25,7 +25,7 @@ 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.
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.
}
componentWillReceiveProps (nextProps) {
// case where componentDidMount triggered new props
if (requestIsNewRequest(nextProps, 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 (requestIsAnAssetRequest(nextProps) && requestIsNewRequest(nextProps, 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.
const { requestId, requestName, requestModifier, assetRequests } = nextProps;
const existingRequest = assetRequests[requestId];
if (existingRequest) { // case: the assetRequest exists
@ -34,7 +34,7 @@ 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.
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.
this.onNewRequest(requestId, requestName, requestModifier);
}
} else {
console.log('show.assetRequestId did not update');
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.
console.log('ShowAsset receiving new props -> request.id did not update', nextProps);
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.
}
}
onNewRequest (id, 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.

View file

@ -30,8 +30,8 @@ const mapDispatchToProps = dispatch => {
onShowNewChannel: (id, channelData) => {
dispatch(showNewChannel(id, channelData));
},
onShowExistingChannel: (error, name, shortId, longId, claimData) => {
dispatch(updateShowChannel(error, name, shortId, longId, claimData));
onShowExistingChannel: (error, name, shortId, longId, claimsData) => {
dispatch(updateShowChannel(error, name, shortId, longId, claimsData));
},
onShowChannelClear: () => {
dispatch(clearShowChannel());

View file

@ -32,6 +32,8 @@ class ShowChannel extends React.Component {
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
} else {
this.onNewChannelRequest(requestId, requestChannelName, requestChannelId);
}
} else {
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
console.log('ShowChannel receiving new props -> request.id did not update', nextProps);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
};
}
onNewChannelRequest (requestId, requestName, requestChannelId) {
@ -47,7 +49,7 @@ class ShowChannel extends React.Component {
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
const channelRecordId = `c#${channelData.name}#${channelData.longId}`;
const existingChannel = channelList[channelRecordId];
if (existingChannel) {
this.showExistingChannel(channelRecordId, existingChannel);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
this.showExistingChannel(existingChannel);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
} else {
this.showNewChannel(channelRecordId, channelData);
}
@ -56,8 +58,9 @@ class ShowChannel extends React.Component {
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
this.props.onShowNewChannel(channelRecordId, channelData);
};
showExistingChannel (existingChannel) {
const { error, channelData: {name, shortId, longId}, claimData } = existingChannel;
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
this.props.onShowExistingChannel(error, name, shortId, longId, claimData);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
console.log('showExistingChannel:', existingChannel);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
const { error, channelData: {name, shortId, longId}, claimsData } = existingChannel;
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
this.props.onShowExistingChannel(error, name, shortId, longId, claimsData);
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
};
componentWillUnmount () {
console.log('ShowChannel will unmount');

neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.
neb-b commented 2018-02-13 06:15:15 +01:00 (Migrated from github.com)
Review

Same comments above about previousRequest. I think a more understandable approach would just be:

if (!channel) this.props.onNewChannelRequest(...)
Same comments above about `previousRequest`. I think a more understandable approach would just be: ``` if (!channel) this.props.onNewChannelRequest(...) ```
bones7242 commented 2018-02-14 02:18:19 +01:00 (Migrated from github.com)
Review

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

See above re: previous request. I mostly fixed this, but not sure if more consolidation can be done.

View file

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Hmm, I like the readability of the switch statement, but I might use this util instead. I have to look at the app and see exactly how it works.
neb-b commented 2018-02-09 20:57:10 +01:00 (Migrated from github.com)
Review
Here is an example of it in the app https://github.com/lbryio/lbry-app/blob/master/src/renderer/redux/reducers/shape_shift.js#L99
shortId: null,
longId : null,
},
channelClaimsData: {
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
claimsData: {
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
claims : null,
currentPage: null,
totalPages : 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
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

@ -118,7 +118,8 @@ function* getNewChannelDataAndShowChannel (action) {
// yield put(addNewChannelToChannelList(id, message, null, null));
}
yield put(updateShowChannel(null, name, shortId, longId, claimsData));
yield put(addNewChannelToChannelList(id, null, name, shortId, longId, claimsData));
const channelData = {name, shortId, longId};
yield put(addNewChannelToChannelList(id, null, channelData, claimsData));
}
export function* watchNewAssetRequest () {