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

Removed AutoComplete onBlur handling when user has clicked a menu item #382

Merged
merged 1 commit into from
May 23, 2016

Conversation

noisecapella
Copy link
Contributor

@noisecapella noisecapella commented May 23, 2016

What are the relevant tickets?

Fixes #380

What's this PR do?

When the user selected an item the onBlur handler cleared the item that was selected immediately afterwards. This change causes the onBlur handler not to be invoked if the user just selected a menu item.

Where should the reviewer start?

this.timerTouchTapCloseId is set after the user selects a menu item, so we are using this in the onFocus callback to determine if the loss of focus was caused by clicking a menu item

How should this be manually tested?

We should be thorough in testing all different scenarios. For each item below notice these things:

  • unless you are actively typing there should never be text in the text field that doesn't match the currently selected item.
  • If an item should be selected, the country text field should have the text of the item, and when you click Save, it should pass validation. You should be able to click the state field and see a list of states that match that country.
  • If an item should not be selected, the country and state fields should be blank. When you click Save it should show a validation error under the Country. When you click the state field you should see no states to be selected.

If you're testing this locally you should also be able to verify the redux state matches what the field indicates.

For each item below start by opening a new employment form with the plus button. Then:

  • Click the country field, then click a country in the list. Verify that you selected that country.
  • Click the country field, type some text such that you get a country in the list, then press Enter to select the first country in the list. Verify that you selected the first country in the list.
  • Click on the country field, then type some text such that it doesn't match any countries. Press enter. Verify that no country is selected.
  • Click on the country field, then type text that matches the name of a country, but click elsewhere rather than selecting the country. Verify that no country is selected.
  • Click on the country field, then press down until you get to a country you want to select. Press Enter on that country. Verify that this country is selected.
  • First, select a country. Then click on a previous field, then TAB until you get to the country field. The menu list should not open up and the text should be selected. Press TAB again, then verify that the country is still selected.
  • First, select a country. Click on the field, then click on another field. Verify that the country is still selected.

Also:

  • verify that you see a complete list of countries when you click the field, but when you type text you see only countries whose prefix matches the text
  • edit another work history and verify that the expected country shows up in the select field

Hopefully that covers all the scenarios we care about!

Any background context you want to provide?

Screenshots (if appropriate)

What GIF best describes this PR or how it makes you feel?

@bdero bdero temporarily deployed to micromasters-ci-pr-382 May 23, 2016 13:55 Inactive
@alicewriteswrongs alicewriteswrongs self-assigned this May 23, 2016
@noisecapella
Copy link
Contributor Author

Found a bug: start with the state and country fields selected. Change the country field and you see the state field clear in the UI. However the redux state for the state field doesn't change until you interact with the state field, so it will pass validation despite being a state for a different country.

@alicewriteswrongs
Copy link
Contributor

confirmed, I was able to repro that

@alicewriteswrongs
Copy link
Contributor

alicewriteswrongs commented May 23, 2016

testing scenarios:

  • Click the country field, then click a country in the list. Verify that you selected that country.
  • Click the country field, type some text such that you get a country in the list, then press Enter to select the first country in the list. Verify that you selected the first country in the list.
  • Click on the country field, then type some text such that it doesn't match any countries. Press enter. Verify that no country is selected.
  • Click on the country field, then type text that matches the name of a country, but click elsewhere rather than selecting the country. Verify that no country is selected.
  • Click on the country field, then press down until you get to a country you want to select. Press Enter on that country. Verify that this country is selected.
  • First, select a country. Then click on a previous field, then TAB until you get to the country field. The menu list should not open up and the text should be selected. Press TAB again, then verify that the country is still selected.
  • First, select a country. Click on the field, then click on another field. Verify that the country is still selected.

I'm verifying by doing the UI interaction and then confirming the redux state is correct. Additionally, to verify interactions that are intended to select nothing, I'm confirming that a validation error pops up when I 'save and continue'.

@alicewriteswrongs
Copy link
Contributor

OK so besides the bug pointed out above things seem to be working correctly to me!

@noisecapella
Copy link
Contributor Author

I'm not sure why the tests are failing. I'm just going to run them again and hope for the best

I filed the bug I mentioned as #383

@alicewriteswrongs
Copy link
Contributor

cool, let's merge this and take care of that later.

assuming the tests pass (maybe we should run them twice since they failed once) 👍

@noisecapella
Copy link
Contributor Author

I see the comment you made in my email but not on github... 😨

I think the tests on Travis are just timing out because it's unbelievably slow right now

@alicewriteswrongs
Copy link
Contributor

OK github comments still don't seem 💯%, but this is 👍 from me once the tests pass reliably.

@alicewriteswrongs
Copy link
Contributor

ahaha and then they reappear 😱

@bdero bdero temporarily deployed to micromasters-ci-pr-382 May 23, 2016 16:48 Inactive
@noisecapella noisecapella added this to the Yo-Yo milestone May 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: in employment form, selecting a country needs to clear the state/territry field
3 participants