search

gdotdesign / elm-ui

UI library for making web applications with Elm
https://elm-ui.netlify.com
BSD 2-Clause "Simplified" License
920 stars 39 forks source link

clear button for Input & SearchInput #42

Closed eimfach closed 7 years ago

eimfach commented 7 years ago

This PR should resolve issue #32 . Hello, this is my first contribution to an elm project. I did run the tests for chrome which passed (firefox seems not yet supported - missing geckodriver ? In my firefox tests mostly everything failed). Also, I wasn't sure which branch this PR to target for so I just used master.

The clear button works like that: it only appears if there is text and the input is not readonly or disabled.

Very good project structure, was very nice to work with. I would also try to resolve further issues.

Greetings

gdotdesign commented 7 years ago

Thanks for this PR, really appreciate it :smile:

If you are willing to put some more work into it, I would suggest putting this feature under a flag in the model, something like showClearIcon : Bool and not make it show up by default.

There is no branching model at this point, so master is fine, I'm transitioning to use development but it's now contains the code for 0.5.0 with some breaking changes, after that we'll probably use it for PRs.

As for the tests, as you can see they are quite sparse and fail regularly for no reason, so I'm not really worried about them at this point. They should work with Firefox though, maybe the selenium package or your Firefox might be outdated, this would probably causes it to fail.

eimfach commented 7 years ago

Thanks for your feedback, appreciate it too :) I added the feature toggle you mentioned. Should it be able to be set on init ? As for now it just lives in the Model and needs to be enabled afterwards via update. I set the default to False.

gdotdesign commented 7 years ago

It's fine now, thanks :)

I'm moving away from setting thins in init, in 0.5.0 I'm adding functions for these kind of things so it would look something like this:

input = 
  Ui.Input.init "defaultValue"
    |> Ui.Input.placeholder "Some text..."
    |> Ui.Input.showClearIcon True

where the showClearIcon would be:

showClearIcon : Bool -> Model -> Model
showClearIcon value model =
  { model | showClearIcon = value }

If you like you can put it in there now, or I'll can do it later.