Skip to content

Fixes #1609 - remove logic for validating problem type from description…#1633

Merged
miketaylr merged 1 commit intomasterfrom
issues/1609/2
Jul 6, 2017
Merged

Fixes #1609 - remove logic for validating problem type from description…#1633
miketaylr merged 1 commit intomasterfrom
issues/1609/2

Conversation

@cch5ng
Copy link
Copy Markdown
Contributor

@cch5ng cch5ng commented Jul 3, 2017

… field interaction

tests: #1609; remove test for problem type validation; add test for description validation

@cch5ng cch5ng force-pushed the issues/1609/2 branch 2 times, most recently from 8389ae7 to 8188593 Compare July 3, 2017 01:24
… field interaction

tests: #1609; remove test for problem type validation; add test for description validation

tests: #1574; update test to assume just one error message results

tests: #1609; try use window helpers as in url test; possible timing issue when using css selector and click to change focus

style: #1609; linting fix
@cch5ng
Copy link
Copy Markdown
Contributor Author

cch5ng commented Jul 3, 2017

@miketaylr R?

(update) I think the only functional difference here is that there isn't a really a trigger for the problem type radio buttons validation error message to display.

(update 2) p.s. I've actually had in mind a different flow for the form's required fields validation. the gist:

  • if the user clicks and leaves a required text input field and it is empty, the help error should display
  • the submit button should be active always
    • but when the user clicks the submit button, a validation flow for all required fields should occur (before data is saved)
    • if required fields are empty, help errors should display and the data will not be saved
    • else the data will be saved

I could do a quick prototype of the concept. then people could give comments on it. probably towards mid week.

@miketaylr
Copy link
Copy Markdown
Member

@miketaylr R?

oops. my fail filters are looking for "r? @miketaylr" -- sorry i missed this!

@cch5ng
Copy link
Copy Markdown
Contributor Author

cch5ng commented Jul 6, 2017

@miketaylr oh, sorry. didn't realize that was case sensitive but I'll remember for the future. thanks!

@miketaylr miketaylr self-requested a review July 6, 2017 15:10
@miketaylr
Copy link
Copy Markdown
Member

Cool, thanks @cch5ng!

@miketaylr miketaylr merged commit 57bf119 into master Jul 6, 2017
@miketaylr miketaylr deleted the issues/1609/2 branch July 6, 2017 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants