Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create project and feed tweaks #255

Merged
merged 7 commits into from
Dec 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/common/components/Loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ import Icon from '@conveyal/woonerf/components/icon'
import React, {Component} from 'react'
import { Row, Col } from 'react-bootstrap'

type Props = {}
type Props = {
style?: {[string]: string | number}
}

export default class Loading extends Component<Props> {
render () {
const {style} = this.props
return (
<Row>
<Col xs={12}>
<p className='text-center'>
<Icon className='fa-5x fa-spin' type='refresh' />
<Icon style={style} className='fa-5x fa-spin' type='refresh' />
</p>
</Col>
</Row>
Expand Down
6 changes: 5 additions & 1 deletion lib/index.css
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@

@import url(node_modules/font-awesome/css/font-awesome.css);
@import url(node_modules/leaflet/dist/leaflet.css);
/**
* Note: this css file must be imported so that marker icons URLs will work
* properly with react-leaflet. See https://github.com/PaulLeCam/react-leaflet/issues/453
*/
@import url(https://unpkg.com/leaflet@1.3.4/dist/leaflet.css);

@import url(node_modules/bootstrap/dist/css/bootstrap.min.css);
@import url(node_modules/react-bootstrap-table/dist/react-bootstrap-table.min.css);
Expand Down
35 changes: 28 additions & 7 deletions lib/manager/components/CreateFeedSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import {
Panel,
Row
} from 'react-bootstrap'
import {isUri} from 'valid-url'
import validator from 'validator'

import {createFeedSource} from '../actions/feeds'
import Loading from '../../common/components/Loading'
import {validationState} from '../util'

import type {NewFeed} from '../../types'
Expand All @@ -36,13 +37,29 @@ type Validation = {
}

type State = {
loading?: boolean,
model: NewFeed,
validation: Validation
}

export default class CreateFeedSource extends Component<Props, State> {
componentWillMount () {
this._initializeState(this.props)
window.addEventListener('keydown', this._handleKeyDown)
}

componentWillUnmount () {
window.removeEventListener('keydown', this._handleKeyDown)
}

_handleKeyDown = (e: SyntheticKeyboardEvent<HTMLInputElement>) => {
switch( e.keyCode ) {
case 13: // ENTER
this._onSave()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be some kind of validation check done before calling _onSave. It's possible to create a feed source without a name with this change.

break
default:
break
}
}

componentWillReceiveProps (nextProps: Props) {
Expand Down Expand Up @@ -77,14 +94,16 @@ export default class CreateFeedSource extends Component<Props, State> {
)

_onSave = () => {
const {model} = this.state
if (model.url === '') {
delete model.url
}
const {model, validation} = this.state
// Prevent a save if the form has validation issues
if (!validation._form) return
// Ensure that URL with empty string literal is not saved to server.
if (model.url === '') delete model.url
model.retrievalMethod = model.autoFetchFeed
? 'FETCHED_AUTOMATICALLY'
: 'MANUALLY_UPLOADED'
this.props.createFeedSource(model)
this.setState({loading: true})
}

_toggleCheckBox = memoize((fieldName: string) => () => {
Expand All @@ -99,14 +118,15 @@ export default class CreateFeedSource extends Component<Props, State> {
const validation: Validation = {
_form: false,
name: !(!model.name || model.name.length === 0),
url: model.url === '' || !!isUri(model.url)
url: !model.url || validator.isURL(model.url)
}
validation._form = !!(validation.name && validation.url)
this.setState({ validation })
}

render () {
const {model, validation} = this.state
const {loading, model, validation} = this.state
if (loading) return <Loading style={{marginTop: '30px'}} />
return (
<Row>
<Col xs={12}>
Expand All @@ -122,6 +142,7 @@ export default class CreateFeedSource extends Component<Props, State> {
>
<ControlLabel>Feed source name</ControlLabel>
<FormControl
autoFocus
value={model.name}
name={'name'}
onChange={this._onInputChange('name')}
Expand Down
3 changes: 2 additions & 1 deletion lib/manager/components/CreateProject.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export default class CreateProject extends Component<Props> {
<h2>Create New Project</h2>
<ProjectSettingsForm
onCancelUrl='/project'
project={{ name: '' }}
// Initialize project with empty object.
project={{}}
updateProject={this._saveProject}
/>
</Col>
Expand Down
1 change: 1 addition & 0 deletions lib/manager/components/FeedSourceSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export default class FeedSourceSettings extends MessageComponent<Props, State> {
<ControlLabel>Feed source name</ControlLabel>
<InputGroup>
<FormControl
autoFocus
value={name || feedSource.name}
name={'name'}
disabled={disabled}
Expand Down
77 changes: 45 additions & 32 deletions lib/manager/components/ProjectSettingsForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@ export default class ProjectSettingsForm extends MessageComponent<Props, State>

componentWillMount () {
this._updateStateFromProps(this.props)
window.addEventListener('keydown', this._handleKeyDown)
}

componentWillUnmount () {
window.removeEventListener('keydown', this._handleKeyDown)
}

_handleKeyDown = (e: SyntheticKeyboardEvent<HTMLInputElement>) => {
switch( e.keyCode ) {
case 13: // ENTER
this._onSaveSettings()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be some kind of validation check done before calling _onSaveSettings. It's possible to create a project without a name with this change.

break
default:
break
}
}

componentWillReceiveProps (nextProps: Props) {
Expand Down Expand Up @@ -208,17 +223,11 @@ export default class ProjectSettingsForm extends MessageComponent<Props, State>
}

_onChangeName = ({target}: {target: HTMLInputElement}) => {
const value = target.value
const {name, value} = target
this.setState(
update(this.state, {
model: {
$merge: {[target.name]: value}
},
validation: {
name: {
$set: value && value.length > 0
}
}
model: { $merge: {[name]: value} },
validation: { [name]: { $set: value && value.length > 0 } }
})
)
}
Expand Down Expand Up @@ -289,32 +298,37 @@ export default class ProjectSettingsForm extends MessageComponent<Props, State>
}

_onSaveSettings = () => {
// if updating, only the things that have changed should be sent to the server
const {project, updateProject} = this.props
// Prevent a save if there have been no edits or form is invalid
if (this._settingsAreUnedited() || !this._formIsValid()) return
// Only the things that have changed should be sent to the server. This avoids
// persisting JSON properties derived from a Jackson method.
updateProject(project.id || '', this._getChanges(), true)
}

_getChanges = () => {
const {model} = this.state
let changes: any
if (this.props.project.id) {
// updating a project
changes = {}
Object.keys(model).map(k => {
if (model[k] !== this.props.project[k]) {
changes[k] = model[k]
}
})
} else {
// creating a project
changes = model
}
this.props.updateProject(this.props.project.id || '', changes, true)
const {project} = this.props
let changes: any = {}
Object.keys(model).map(k => {
if (model[k] !== project[k]) {
changes[k] = model[k]
}
})
return changes
}

_settingsAreUnedited = () => Object.keys(this.state.model).length === 0 &&
this.state.model.constructor === Object
_settingsAreUnedited = () => Object.keys(this._getChanges()).length === 0

_formIsValid = () => {
const {validation} = this.state
return Object.keys(validation).every(k => validation[k])
}

render () {
const {editDisabled, showDangerZone} = this.props
const {model, validation} = this.state
const {autoFetchHour, autoFetchMinute} = model
const noEdits = Object.keys(model).length === 0 && model.constructor === Object
const autoFetchChecked = model.autoFetchFeeds
if (editDisabled) {
return (
Expand All @@ -336,7 +350,8 @@ export default class ProjectSettingsForm extends MessageComponent<Props, State>
>
<ControlLabel>{this.messages('fields.name')}</ControlLabel>
<FormControl
value={model.name}
autoFocus
value={model.name || ''}
name={'name'}
onChange={this._onChangeName}
/>
Expand Down Expand Up @@ -482,10 +497,8 @@ export default class ProjectSettingsForm extends MessageComponent<Props, State>
data-test-id='project-settings-form-save-button'
disabled={
editDisabled ||
noEdits ||
!validation.name ||
!model.name ||
model.name === ''
this._settingsAreUnedited() ||
!this._formIsValid()
}
onClick={this._onSaveSettings}>
{this.messages('save')}
Expand Down
12 changes: 8 additions & 4 deletions lib/manager/components/ProjectViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,14 @@ export default class ProjectViewer extends MessageComponent<Props, State> {
<span className='hidden-xs'>{this.messages('settings')}</span>
</span>
}>
<ProjectSettings
activeSettingsPanel={activeSubComponent}
projectEditDisabled={projectEditDisabled}
{...this.props} />
{// Prevent rendering component if not active to ensure that
// keyboard listener is not active while form is not visible.
activeComponent === 'settings' &&
<ProjectSettings
activeSettingsPanel={activeSubComponent}
projectEditDisabled={projectEditDisabled}
{...this.props} />
}
</Tab>
</Tabs>
</Grid>
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@
"turf-point-on-line": "^3.0.11",
"turf-polygon": "^1.0.3",
"uuid": "^3.1.0",
"valid-url": "^1.0.9",
"validator": "7.0.0"
},
"devDependencies": {
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11707,11 +11707,6 @@ uuid@^3.0.0, uuid@^3.3.2:
resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.3.2.tgz#1b4af4955eb3077c501c23872fc6513811587131"
integrity sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==

valid-url@^1.0.9:
version "1.0.9"
resolved "https://registry.yarnpkg.com/valid-url/-/valid-url-1.0.9.tgz#1c14479b40f1397a75782f115e4086447433a200"
integrity sha1-HBRHm0DxOXp1eC8RXkCGRHQzogA=

validate-npm-package-license@^3.0.1:
version "3.0.3"
resolved "https://registry.yarnpkg.com/validate-npm-package-license/-/validate-npm-package-license-3.0.3.tgz#81643bcbef1bdfecd4623793dc4648948ba98338"
Expand Down