search

guylabs / ion-autocomplete

A configurable Ionic directive for an autocomplete dropdown
MIT License
264 stars 88 forks source link

Added ClearOnRemove functionality #232

Open xam8re opened 7 years ago

xam8re commented 7 years ago

For customer request, I've implemented the closing of modal when a single item are removed.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.07%) to 73.37% when pulling 170a9ae52d93a22baa66f6cf6ea1b07fa4428ead on xam8re:master into ae104803f10f03c40dac4d0fcf78e7e90aba4194 on guylabs:master.

guylabs commented 7 years ago

Hi @xam8re

thanks a lot for the contribution but I have a question and some comments before I can do the code review:

  1. The requirement is that when a single item is selected and the user clicks on the remove item it will clear the item and close the modal right?
  2. Can you please use the hideModal() method to close the modal instead of the cancelClick()? As the cancel click should be used then the user presses the cancel button and not when we want to close the modal in another method.
  3. Could you remove the whitespace changes in your commit?
  4. Can you maybe add a simple end to end test (see the tests already implemented).
  5. Please execute the grunt build task and update the pull request with the generated files.

When you did I can check the pull request again and do a review. If you have issues some of these points please don't hesitate to ask me.

Thanks a lot and regards

Guy

xam8re commented 7 years ago

Hi @guylabs shure. I've done the edit using modal close. I need implement the test but, under windows it doesn't work. As soon as I've a it of time I'll test under linux and I commit everithings.

guylabs commented 7 years ago

Ok nice! What does not work in Windows? What is the error? Maybe I can help you fix it.

RodrigoSFC commented 7 years ago

Good Morning, When you are using the browser and the input is in the middle of the page, when you click on the input, it will launch the OnMouseDown when the mouse is over an option that can be selected, and the OnMouseDown event ends up selecting the item.

Could they correct?

Thanks for the plugin, it is very good! Best Regards Rodrigo.