React/Redux - publish component #323

Merged
bones7242 merged 80 commits from react-upload into master 2018-01-25 22:43:20 +01:00
7 changed files with 14 additions and 12 deletions
Showing only changes of commit 04754920df - Show all commits

View file

@ -1,9 +1,9 @@
import React from 'react';
import PropTypes from 'prop-types';
function UrlMiddle ({publishInChannel, loggedInChannelName, loggedInChannelShortId}) {
function UrlMiddle ({publishInChannel, selectedChannel, loggedInChannelName, loggedInChannelShortId}) {
if (publishInChannel) {
if (loggedInChannelName) {
if (selectedChannel === loggedInChannelName) {
return <span id="url-channel" className="url-text--secondary">{loggedInChannelName}:{loggedInChannelShortId} /</span>;
}
return <span id="url-channel-placeholder" className="url-text--secondary tooltip">@channel<span

View file

@ -1,7 +1,6 @@
import {connect} from 'react-redux';
import {setPublishInChannel} from 'actions/publish';
import {setPublishInChannel, updateSelectedChannel} from 'actions/publish';
import View from './view.jsx';
import {updateSelectedChannel} from '../../actions/publish';
const mapStateToProps = ({ channel, publish }) => {
return {

View file

@ -1,7 +1,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?
import React from 'react';
import ChannelLoginForm from 'containers/ChannelLoginForm';
import ChannelCreateForm from 'containers/ChannelCreateForm';
import * as states from 'constants/channel_select_states';
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?
import * as states from 'constants/publish_channel_select_states';
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?
class ChannelSelect extends React.Component {
constructor (props) {

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

@ -8,6 +8,7 @@ const mapStateToProps = ({ channel, publish }) => {
loggedInChannelShortId: channel.loggedInChannel.shortId,
fileName : publish.file.name,
publishInChannel : publish.publishInChannel,
selectedChannel : publish.selectedChannel,
claim : publish.claim,
urlError : publish.error.url,
};

View file

@ -5,10 +5,6 @@ import UrlMiddle from 'components/PublishUrlMiddleDisplay';
class PublishUrlInput extends React.Component {
constructor (props) {
super(props);
this.state = {
host : 'spee.ch',
urlMiddle: null,
};
this.handleInput = this.handleInput.bind(this);
this.cleanseInput = this.cleanseInput.bind(this);
this.setClaimNameFromFileName = this.setClaimNameFromFileName.bind(this);
@ -65,9 +61,14 @@ class PublishUrlInput extends React.Component {
<label className="label">URL:</label>
</div><div className="column column--7 column--sml-10 input-text--primary span--relative">
<span className="url-text--secondary">{this.state.host} / </span>
<span className="url-text--secondary">spee.ch / </span>
<UrlMiddle publishInChannel={this.props.publishInChannel} loggedInChannelName={this.props.loggedInChannelName} loggedInChannelShortId={this.props.loggedInChannelShortId}/>
<UrlMiddle
publishInChannel={this.props.publishInChannel}
selectedChannel={this.props.selectedChannel}
loggedInChannelName={this.props.loggedInChannelName}
loggedInChannelShortId={this.props.loggedInChannelShortId}
/>
<input type="text" id="claim-name-input" className="input-text" name='claim' placeholder="your-url-here" onChange={this.handleInput} value={this.props.claim}/>
{ (this.props.claim && !this.props.urlError) && <span id="input-success-claim-name" className="info-message--success span--absolute">{'\u2713'}</span> }

View file

@ -1,8 +1,9 @@
import * as actions from 'constants/publish_action_types';
import { LOGIN } from 'constants/publish_channel_select_states';
const initialState = {
publishInChannel : false,
selectedChannel : null,
selectedChannel : LOGIN,
showMetadataInputs: false,
status : {
status : null,