Switch to Mobile-Detect.json as base for gem

[johnnyshields]

It would be nice if the is_tablet/is_mobile detection logic could be moved to the https://github.com/josh/useragent gem, so there's one "golden source" for this. The mobile-fu gem could then use useragent as a dependency for it's extra view template magic.

Just throwing the idea out there. @josh @benlangfeld

[benlangfeld]

useragent does not claim to provide anything of the "is this mobile or not?" functionality. Currently mobile-fu relies on rack-mobile-detect for this.

Further thoughts (including from @josh) would be appreciated, but right now I'm not convinced of the benefit of this change.

[serbanghita]

May I suggest in using the logic behind Mobile_Detect class https://github.com/serbanghita/Mobile-Detect/blob/master/Mobile_Detect.json
With every release we export the new properties there. You could focus entirely on other features and leverage the detection strategy. (sounds corporate 😄 )

Any thoughts on this?

[johnnyshields]

👍 +1 for reading in the Mobile_Detect.json file. Great suggestion, much better than maintaining idiosyncratic logic inside the mobile-fu code.

[johnnyshields]

@benlangfeld would you accept a PR to implement MobileDetect JSON as the base for mobile detection? Looking at their Github page seems they are widely implemented across many platforms, and I'm sure @serbanghita would advertise "Mobile-Fu" as the Ruby/Rails lib for MobileDetect (looks like there is none currently).

[benlangfeld]

I already did accept this at #43. I've submitted a request to link back to us at serbanghita/Mobile-Detect#236. I would accept additions to our README explaining that we use Mobile-Detect.

I think this issue is satisfied, so I'll close it for now.

[benlangfeld]

Actually, now I remember our position. We're not using MobileDetect at runtime, only to assist in development of this gem. I would accept a pull request that updated us at runtime when loading the gem (not on every Rails request) as long as it had a static fallback.

[johnnyshields]

(Renaming issue for clarity)

@benlangfeld I'd propose:

• we keep a copy of Mobile-Detect.json within this library, which is updated periodically by copy/paste
• we parse the file once at the time the gem is bootstrapped via Ruby's built-in JSON lib, and convert it to a frozen regex variable internally (replacing the existing regex vars)

What do you think?

[johnnyshields]

I also don't think we need a static fallback (the JSON file is static). If for some reason the JSON is broken, the unit tests (Travis CI) will fail equivalently if a Ruby file was broken--in other words a .json file should be every bit as reliable as an .rb file.

[johnnyshields]

Lastly, we can modify the existing Rake task from #43 so that it simply copies the file from Mobile-Detect lib and overwrites the existing file (saving us the pain of copying and pasting)

[benlangfeld]

I'm happy with that as an initial step. My comments were on the assumption that we would fetch Mobile-Detect.json at runtime.

Perhaps we could do it in these steps:

1. Include Mobile-Detect.json in mobile-fu, parsed at gem include time
2. Allow providing an alternative Mobile-Detect.json from the application via an initializer
3. Fetching Mobile-Detect.json at runtime, falling back to 2 and then 1).

I'm happy with all of your other comments.

[johnnyshields]

@benlangfeld I will go ahead with this PR then.

Re: 2, I don't think there is a need for another lib here, we should encourage any fixes for devices to be raised as PRs for Mobile-Detect itself and keep it as a "golden source". That being said, from time-to-time new devices will be released and Mobile-Detect may lag behind. For this reason, we could allow users to specify a custom location from where to load their own manually edited Mobile-Detect.json file (key point being the format is "Mobile-Detect.json").

Re: 3, I personally would prefer not to fetch Mobile-Detect.json at runtime, as I like to keep everything vetted and stable (i.e. passing in Travis) in production. But if other users want it, they are welcome to raise it as a separate PR (as long as I can disable it 😄)

[benlangfeld]

Happy with your comments on 3, not essential at all.

As 2, I didn't mean any other library, I mean a Rails application itself. This way an individual application may stay more or less up to date than mobile-fu, or customise rules. Your comments are exactly what I was suggesting :) I would not accept updates to Mobile-Detect.json included in mobile-fu other than to match a release of Mobile-Detect's PHP implementation.