arvindvyas / Country-State-Select

It will fetch the countries and according to that fetch the state of that countries, currently it is for countries listing
MIT License
69 stars 78 forks source link

New API + Documentation Created #20

Closed aldefouw closed 9 years ago

aldefouw commented 9 years ago

Hi Arvind -

First of all - great job on this gem! It was really inspiring, and it was useful for my project.

I decided to make some improvements to it and release a new version of it on the fork of my branch. It's working great for me on my project.

I'm wondering if you might take a look at it and consider merging it into your master branch. (Perhaps with some changes?)

What I've done is the following:

1) Introduced a new JavaScript API, which is listed in my documentation.

One method call, which sets the IDs of the country and states fields, can create the integration you need to create the dynamic States / Provinces dropdown.

2) Made the CountryStateSelect public methods easier.

Basically, for countries you use:

CountryStateSelect.countries_collection

For states, you use:

CountryStateSelect.states_collection(f, options)

That's all you need in the collection object of the input field object.

3) Added in rspec unit tests on the new methods that I added.

You can see them in the /spec/ folder.

4) Added a test / dummy application that integrates the functionality.

We could use this to write cucumber tests that would actually look at how it works functionally in the browser.

5) Added a thorough description of how to implement.

Documentation is tough to write, so it can be improved. But I think it would be helpful for people to have the documentation that I included in my version.

6) Added in gem dependencies:

This might limit who can use the gem, but I think most folks who use gems are probably using Rails anyway. The other gems are useful in their own right.

I am not 100% sure if you NEED simple_form to implement this gem. Haven't tested it yet. I suspect it would work without simple_form, but it will be installed as an option in the background.

The same goes for chosen-rails. It isn't 100% necessary for the gem to be useful, but it makes life easier for the end user as far as user interface is concerned.

Final thoughts ...

This is my first shot at a gem, so perhaps there are some things that I messed up and / or missed.

If you have any interest in using this updated version for your gem, feel free. It has been a fun learning experience for me over the past few days.

I'm pretty busy but perhaps I could help you maintain this, too. It shouldn't be too bad if we implement some more tests.

Look forward to hearing from you!

Sincerely, Adam De Fouw

arvindvyas commented 9 years ago

Hi Aldefouw

Great job , I am looking in to it , but I can not merge it currently because build is failed so please have a look in to it

failing

It should pass as you can check here https://travis-ci.org/arvindvyas/Country-State-Select/builds/74101764.

Thanks Arvind

aldefouw commented 9 years ago

Hi Arvind -

Ah - yes, I saw that last night. Like I said, I'm new to this - so I haven't really seen the CI tests before.

Are the CI Travis tests basically testing to make sure that bundle install works?

Because the confusing thing is that I've written this for Ruby 2.0 + but it looks like it is failing on everything except 1.9.3, which is a bit confusing.

Anyway, I can look into it tonight. =) At my "day job" right now.

Adam D.

aldefouw commented 9 years ago

Okay - that was pretty easy to fix actually.

The problem was that the bundler version didn't seem to work with the Ruby 2.1.2 version.

2.0.0 failed because it couldn't resolve the hostname "github.com" which was odd. I think it was a fluke.

Oh - FYI - I also added in my RSpec tests to the .travis.yml file now that I understand what it is. =)

In the future, we could theoretically add in Cucumber tests as well =).

Hopefully now that tests are passing it's ready to go! Please let me know if there are any additional concerns.

Adam De Fouw