React router #343

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

View file

@ -20,7 +20,7 @@ export function getLongClaimId (name, modifier) {
}), }),
body: JSON.stringify(body), body: JSON.stringify(body),
} }
// crate url // create url
const url = `/api/claim/long-id`; const url = `/api/claim/long-id`;
// return the request promise // return the request promise
return Request(url, params); return Request(url, params);

View file

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

Why do you do const that = this?

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

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

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

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

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

View file

@ -6,9 +6,9 @@ const mapStateToProps = ({ show }) => {
// select title // select title
const existingRequest = show.assetRequests[show.request.id]; const existingRequest = show.assetRequests[show.request.id];
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`; const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`;
const existingAsset = show.assetList[assetKey]; const asset = show.assetList[assetKey];
if (existingAsset) { if (asset) {
props['title'] = existingAsset.claimData.title; props['title'] = asset.claimData.title;
}; };
return props; return props;
}; };

View file

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

You can use destructuring twice to avoid all the repeated `this.props.claimData` `const { claimData: { name, claimId... } } = this.props` Then you can just use `name={name}`
// select name and claim id // select name and claim id
const existingRequest = show.assetRequests[show.request.id]; const existingRequest = show.assetRequests[show.request.id];
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`; const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`;
const existingAsset = show.assetList[assetKey]; const asset = show.assetList[assetKey];
neb-b commented 2018-02-05 20:34:53 +01:00 (Migrated from github.com)
Review

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

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

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

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

Then you can just use name={name}

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

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

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

Then you can just use name={name}

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

View file

@ -15,10 +15,10 @@ const mapStateToProps = ({ show }) => {
props['existingRequest'] = existingRequest; props['existingRequest'] = existingRequest;
// select asset info // select asset info
const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`; // note: just store this in the request const assetKey = `a#${existingRequest.name}#${existingRequest.claimId}`; // note: just store this in the request
const existingAsset = show.assetList[assetKey]; const asset = show.assetList[assetKey];
if (existingAsset) { if (asset) {
console.log('existing asset found', existingAsset); console.log('existing asset found', asset);
props['asset'] = existingAsset; props['asset'] = asset;
}; };
}; };
return props; return props;

View file

@ -6,6 +6,7 @@ import { getShortId, getClaimData } from 'api/assetApi';
function* getAssetDataAndShowAsset (action) { function* getAssetDataAndShowAsset (action) {
const {id, name, claimId} = action.data; const {id, name, claimId} = action.data;
// get short Id // get short Id
console.log('getting short id');
let success, message, shortId; let success, message, shortId;
try { try {
({success, message, data: shortId} = yield call(getShortId, name, claimId)); ({success, message, data: shortId} = yield call(getShortId, name, claimId));
@ -16,6 +17,7 @@ function* getAssetDataAndShowAsset (action) {
return yield put(updateRequestError(message)); return yield put(updateRequestError(message));
} }
// if no error, get claim data // if no error, get claim data
console.log('getting claim data');
success = null; success = null;
let claimData; let claimData;
try { try {

View file

@ -5,7 +5,6 @@ import { getChannelClaims } from 'api/channelApi';
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

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

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

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

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

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

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

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

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

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

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

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

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
function* getChannelClaimsAndShowChannel (action) { function* getChannelClaimsAndShowChannel (action) {
const { id, name, shortId, longId } = action.data; const { id, name, shortId, longId } = action.data;
console.log('getchannelclaimsandshowchannel', id, name, shortId, longId);
neb-b commented 2018-02-09 22:53:34 +01:00 (Migrated from github.com)
Review

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

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

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

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

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

If these aren't being used anywhere else, I don't think they need to be separated. IMO it would make these saga files easier to understand/follow, but not a big issue
let success, message, claimsData; let success, message, claimsData;
try { try {
({ success, message, data: claimsData } = yield call(getChannelClaims, name, longId, 1)); ({ success, message, data: claimsData } = yield call(getChannelClaims, name, longId, 1));

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

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

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

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

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

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

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

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

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

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

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

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

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