React/Redux - publish component #323

Merged
bones7242 merged 80 commits from react-upload into master 2018-01-25 22:43:20 +01:00
5 changed files with 59 additions and 60 deletions
Showing only changes of commit 41924130a3 - Show all commits

View file

@ -152,6 +152,7 @@ const mapDispatchToProps = dispatch => {
return {
onFileSelect: (file) => {
dispatch(selectFile(file));
dispatch(updateError('publishSubmit', null));
},
onFileError: (value) => {
dispatch(updateError('file', value));

View file

@ -125,13 +125,13 @@ class PublishForm extends React.Component {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
.then(() => {
const metadata = that.createMetadata();
// publish the claim
return that.makePublishRequest(this.props.file, metadata);
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
return that.makePublishRequest(that.props.file, metadata);
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
})
.then(() => {
that.props.onPublishStatusChange('publish request made');
})
.catch((error) => {
that.props.onPublishRequestError(error.message);
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
that.props.onPublishSubmitError(error.message);
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
});
}
render () {
@ -174,8 +174,8 @@ class PublishForm extends React.Component {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
<PublishMetadataInputs />
</div>
<div className="row row--padded row--wide align-content-center">
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
<p className="info-message-placeholder info-message--failure">{this.props.publishRequestError}</p>
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
<div className="row row--padded row--no-top row--wide align-content-center">
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
<p className="info-message-placeholder info-message--failure">{this.props.publishSubmitError}</p>
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
<button id="publish-submit" className="button--primary button--large" onClick={this.publish}>Publish</button>
</div>
@ -207,7 +207,7 @@ const mapStateToProps = state => {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
publishInChannel : state.publishInChannel,
fileError : state.error.file,
urlError : state.error.url,
publishRequestError: state.error.publishRequest,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
publishSubmitError: state.error.publishSubmit,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
};
};
@ -225,8 +225,8 @@ const mapDispatchToProps = dispatch => {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
onPublishStatusChange: (status, message) => {
dispatch(updatePublishStatus(status, message));
},
onPublishRequestError: (value) => {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
dispatch(updateError('publishRequest', value));
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
onPublishSubmitError: (value) => {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
dispatch(updateError('publishSubmit', value));
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
},
};
};
@ -243,12 +243,12 @@ PublishForm.propTypes = {
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
publishInChannel : PropTypes.bool.isRequired,
fileError : PropTypes.string,
urlError : PropTypes.string,
publishRequestError : PropTypes.string,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
publishSubmitError : PropTypes.string,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
onFileSelect : PropTypes.func.isRequired,
onFileClear : PropTypes.func.isRequired,
onChannelLogin : PropTypes.func.isRequired,
onPublishStatusChange: PropTypes.func.isRequired,
onPublishRequestError: PropTypes.func.isRequired,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
onPublishSubmitError: PropTypes.func.isRequired,
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
};
export default connect(mapStateToProps, mapDispatchToProps)(PublishForm);

kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux
kauffj commented 2018-01-15 20:25:38 +01:00 (Migrated from github.com)
Review

These consts need to be shared across files

These consts need to be shared across files
kauffj commented 2018-01-15 20:31:56 +01:00 (Migrated from github.com)
Review

I suspect this shouldn't be necessary with addition of Redux

I suspect this shouldn't be necessary with addition of Redux

View file

@ -45,13 +45,10 @@ class MetadataInputs extends React.Component {
}
render () {
return (
<div>
<div className="column column--10">
<div id="publish-details" className="row row--padded row--no-top row--wide">
<a className="label link--primary" id="publish-details-toggle" href="#" onClick={this.toggleShowInputs}>{this.state.showInputs ? '[less]' : '[more]'}</a>
</div>
{this.state.showInputs && (
<div id="publish-details" className="row row--padded row--wide">
<div>
<div className="row row--no-top">
<div className="column column--3 column--med-10 align-content-top">
<label htmlFor="publish-license" className="label">Description:</label>

View file

@ -53,13 +53,13 @@ class UrlChooser extends React.Component {
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
makeGetRequest(`/api/claim-is-available/${claim}`)
.then(response => {
if (response) {
this.props.onUrlError(null);
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
that.props.onUrlError(null);
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
} else {
this.props.onUrlError('That url has already been claimed');
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
that.props.onUrlError('That url has already been claimed');
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
}
})
.catch((error) => {
this.props.onUrlError(error.message);
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
that.props.onUrlError(error.message);
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
});
}
render () {
@ -97,6 +97,7 @@ const mapDispatchToProps = dispatch => {
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
return {
onClaimChange: (value) => {
dispatch(updateClaim(value));
dispatch(updateError('publishSubmit', null));
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
},
onUrlError: (value) => {
dispatch(updateError('url', value));

kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).
kauffj commented 2018-01-15 20:29:33 +01:00 (Migrated from github.com)
Review

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

You should look at some of the linting additions @IGassman has added to app, could help keep these imports consistent in path specification (among other improvements).

View file

@ -17,7 +17,7 @@ const initialState = {
kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).
kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).
error: {
file : null,
url : null,
publishRequest: null,
kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).
publishSubmit: null,
kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).
},
file : null,
claim : '',

kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).
kauffj commented 2018-01-15 20:27:22 +01:00 (Migrated from github.com)
Review

import * as

`import * as`
kauffj commented 2018-01-15 20:28:40 +01:00 (Migrated from github.com)
Review

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).

Some of these should possibly be renamed or refactored into separate files (or possibly can wait until next refactor/revision).