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 27 additions and 9 deletions
Showing only changes of commit 6a8a908130 - Show all commits

View file

@ -7,6 +7,7 @@ const mapStateToProps = ({ channel, publish }) => {
loggedInChannelName: channel.loggedInChannel.name, loggedInChannelName: channel.loggedInChannel.name,
publishInChannel : publish.publishInChannel, publishInChannel : publish.publishInChannel,
selectedChannel : publish.selectedChannel, selectedChannel : publish.selectedChannel,
channelError : publish.error.channel,
}; };
}; };

View file

@ -24,6 +24,7 @@ class ChannelSelect extends React.Component {
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
render () { render () {
return ( return (
<div> <div>
<p id="input-error-channel-select" className="info-message-placeholder info-message--failure">{this.props.channelError}</p>
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
<form> <form>
<div className="column column--3 column--med-10"> <div className="column column--3 column--med-10">
<input type="radio" name="anonymous-or-channel" id="anonymous-radio" className="input-radio" value="anonymous" checked={!this.props.publishInChannel} onChange={this.toggleAnonymousPublish}/> <input type="radio" name="anonymous-or-channel" id="anonymous-radio" className="input-radio" value="anonymous" checked={!this.props.publishInChannel} onChange={this.toggleAnonymousPublish}/>
@ -36,7 +37,6 @@ class ChannelSelect extends React.Component {
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
</form> </form>
{ this.props.publishInChannel && ( { this.props.publishInChannel && (
<div> <div>
<p id="input-error-channel-select" className="info-message-placeholder info-message--failure">{this.props.channelError}</p>
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
<div className="column column--3"> <div className="column column--3">
<label className="label" htmlFor="channel-name-select">Channel:</label> <label className="label" htmlFor="channel-name-select">Channel:</label>
</div><div className="column column--7"> </div><div className="column column--7">

neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?
neb-b commented 2018-01-18 06:19:56 +01:00 (Migrated from github.com)
Review

Same here, this should be moved to componentDidMount

Same here, this should be moved to `componentDidMount`
neb-b commented 2018-01-18 06:24:14 +01:00 (Migrated from github.com)
Review

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state.

I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen.

Whenever you separate the data into two sources it can cause some weird issues.

Instead of reading prop values and adding them to state, you should just use the prop values and not worry about internal state. I try to avoid internal state whenever possible, one exception is the app homepage, where we store if the user can scroll left or right in a card row. But that isn't dependent on any prop value, just by what is currently on the screen. Whenever you separate the data into two sources it can cause some weird issues.
bones7242 commented 2018-01-19 00:57:08 +01:00 (Migrated from github.com)
Review

@seanyesmunt The issue I am struggling with, is that I (think) I need the <select> element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this <select> element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

@seanyesmunt The issue I am struggling with, is that I (think) I need the `<select>` element to be a controlled component, and if so then it seems like it would be best to use internal state rather than the store, since no other component should care what option is selected unless this component decides to update the state of the store. The reason I am setting the state when the component mounts and when it receives props, is that if another component updates the store when a new channel is logged into, then I want this `<select>` element to re-render with that option selected. Any ideas on how I can approach this differently so that I can eliminate internal state while still accomplishing those goals?

View file

@ -14,6 +14,7 @@ const mapStateToProps = ({ channel, publish }) => {
license : publish.metadata.license, license : publish.metadata.license,
nsfw : publish.metadata.nsfw, nsfw : publish.metadata.nsfw,
publishInChannel : publish.publishInChannel, publishInChannel : publish.publishInChannel,
selectedChannel : publish.selectedChannel,
fileError : publish.error.file, fileError : publish.error.file,
urlError : publish.error.url, urlError : publish.error.url,
publishSubmitError: publish.error.publishSubmit, publishSubmitError: publish.error.publishSubmit,
@ -34,6 +35,9 @@ const mapDispatchToProps = dispatch => {
onPublishStatusChange: (status, message) => { onPublishStatusChange: (status, message) => {
dispatch(updatePublishStatus(status, message)); dispatch(updatePublishStatus(status, message));
}, },
onChannelSelectionError: (value) => {
dispatch(updateError('channel', value));
},
onPublishSubmitError: (value) => { onPublishSubmitError: (value) => {
dispatch(updateError('publishSubmit', value)); dispatch(updateError('publishSubmit', value));
}, },

View file

@ -10,11 +10,24 @@ import * as publishStates from 'constants/publish_claim_states';
class PublishForm extends React.Component { class PublishForm extends React.Component {
constructor (props) { constructor (props) {
super(props); super(props);
this.validatePublishRequest = this.validatePublishRequest.bind(this); this.validateChannelSelection = this.validateChannelSelection.bind(this);
this.validatePublishParams = this.validatePublishParams.bind(this);
this.makePublishRequest = this.makePublishRequest.bind(this); this.makePublishRequest = this.makePublishRequest.bind(this);
this.publish = this.publish.bind(this); this.publish = this.publish.bind(this);
} }
validatePublishRequest () { validateChannelSelection () {
// make sure all required data is provided
return new Promise((resolve, reject) => {
// if publishInChannel is true, is a channel selected & logged in?
if (this.props.publishInChannel && (this.props.selectedChannel !== this.props.loggedInChannel.name)) {
// update state with error
this.props.onChannelSelectionError('Select "Anonymous" or log in to a channel');
// reject this promise
return reject(new Error('Fix the channel'));
}
});
}
validatePublishParams () {
// make sure all required data is provided // make sure all required data is provided
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
// is there a file? // is there a file?
@ -28,10 +41,6 @@ class PublishForm extends React.Component {
if (this.props.urlError) { if (this.props.urlError) {
return reject(new Error('Fix the url')); return reject(new Error('Fix the url'));
} }
// if publishInChannel is true, is a channel logged in (or selected)
if (this.props.publishInChannel && !this.props.loggedInChannel.name) {
return reject(new Error('Select "Anonymous" or log in to a channel'));
}
// is the claim available? // is the claim available?
resolve(); resolve();
}); });
@ -85,7 +94,7 @@ class PublishForm extends React.Component {
thumbnail : this.props.thumbnail, thumbnail : this.props.thumbnail,
}; };
if (this.props.publishInChannel) { if (this.props.publishInChannel) {
metadata['channelName'] = this.props.loggedInChannel.name; metadata['channelName'] = this.props.selectedChannel;
} }
return metadata; return metadata;
} }
@ -103,7 +112,10 @@ class PublishForm extends React.Component {
publish () { publish () {
// publish the asset // publish the asset
const that = this; const that = this;
this.validatePublishRequest() this.validateChannelSelection()
.then(() => {
return that.validatePublishRequest();
})
.then(() => { .then(() => {
const metadata = that.createMetadata(); const metadata = that.createMetadata();
// publish the claim // publish the claim

View file

@ -12,6 +12,7 @@ const initialState = {
error: { error: {
file : null, file : null,
url : null, url : null,
channel : null,
publishSubmit: null, publishSubmit: null,
}, },
file : null, file : null,