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

UX changes for undesirable behaviors and remove top level "All assertions are met" radio option #319

Merged
merged 3 commits into from
Dec 11, 2020

Conversation

spectranaut
Copy link
Contributor

@spectranaut spectranaut commented Nov 4, 2020

Preview Tests

Example test

To test validation:

  1. Select "yes there are undesirable behaviors" for any command for any tests
  2. Click the "submit results" button at the bottom of the page
  3. You should see "required" by the "undesirable results" fieldset legend for the command in step (1)
  4. Select "other"
  5. Click the "submit results" button at the bottom of the page
  6. You should see "required" by the "undesirable results" for the "other" input for the command

To test tab stop changes:

  1. Select "yes there are undesirable changes"
  2. Tab into the undesirable checkbox list
  3. A second tab should move you out of the the undesirable checkbox list
  4. Shift-tab back in
  5. Use the arrow keys to move between the undesirable checkboxes
  6. Tab out and tab back, the last checkbox with focus should be in the tab sequence

@evmiguel
Copy link
Contributor

evmiguel commented Nov 9, 2020

@spectranaut when I click, "Yes, there are undesirable behaviors", only the "Other" field is enabled. Is that what is supposed to happen?

https://www.loom.com/share/7c04a7821e57465f840ef70295c71916

@spectranaut spectranaut force-pushed the fix-undesirable-validation branch 2 times, most recently from 44906b5 to 5a5bbdf Compare November 19, 2020 04:11
@spectranaut
Copy link
Contributor Author

@evmiguel I fixed the bug! Can you do another review? Thanks :)

@spectranaut
Copy link
Contributor Author

This PR now has the updates requested in #320

@spectranaut spectranaut changed the title Fix validation for undesirable checkboxes UX changes for undesirable behaviors and remove top level "All assertions are met" radio option Dec 10, 2020
@rmeritz
Copy link
Contributor

rmeritz commented Dec 10, 2020

@spectranaut - Everything worked as described in the new tab behavior except maybe "A second tab should move you out of the undesirable checkbox list". This moved me to the "If other selected please explain:" textbox. I wasn't sure if it was supposed to move me their I kind of assumed according to the instructions I should have moved to the next SR output box instead.

@spectranaut
Copy link
Contributor Author

@rmeritz sorry what you identified is the correct behavior, it moves you out of the checkboxes and into the "other" box. I wasn't sure if this was the exact correct behavior but it is intentional and we can wait until feedback to change it if users don't like this behavior.

@rmeritz rmeritz merged commit adc6230 into master Dec 11, 2020
@rmeritz rmeritz deleted the fix-undesirable-validation branch December 11, 2020 23:36
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.

3 participants