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

Disable Chosen on iOS devices (iPad, iPod, iPhone) #1388

Merged
merged 3 commits into from
Jul 24, 2013

Conversation

koenpunt
Copy link
Collaborator

As a solution (until actual support is implemented?) for #27, #382, #1088 and #1233 I've flagged iOS browsers as unsupported.

@kenearley
Copy link

Seems like a good idea to me.

@kenearley
Copy link

@@ -204,6 +204,8 @@ class AbstractChosen
@browser_is_supported: ->
if window.navigator.appName == "Microsoft Internet Explorer"
return null isnt document.documentMode >= 8
if /iP(ad|od|hone)/.test(window.navigator.userAgent)
return false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two lines up, we're returning null instead of false. Should probably be consistent. (Though probably consistent with false, not null.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd rather we change two lines up to be false. We're returning true by default and I think it's better to assume that a method name like browser_is_supported will return a boolean (or a JavaScript approximation of a boolean).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh - I see @koenpunt's reply below and he's right. We're not returning null, we're returning the value of the comparison. Carry on.

@tjschuck
Copy link
Member

Would close #409 as well.

@koenpunt
Copy link
Collaborator Author

We're not returning null, we're comparing to null ;)

@koenpunt
Copy link
Collaborator Author

Or is isnt short for js if (!...)?

@tjschuck
Copy link
Member

@koenpunt I really have no idea there. That line should probably be rewritten altogether :)

@pfiller
Copy link
Contributor

pfiller commented Jul 23, 2013

On the iPhone, I don't think there should be any question that Chosen should be disabled. The keyboard covers up the list and searching is awkward.

On iPad, it's less cut and dry. When you zoom in on Chosen, I think it's probably better than the default select control. When it's at 1:1, it's probably a slight edge to the native select (but only because of size). I'm not sure we should make a choice like this for a developer when it's totally functional on iPad. People can opt out when they apply Chosen, no?

I'm not sure what to say about Android. I assume the small form factor problem the iPhone presents is also a problem on Android phones, but is there an easy way to separate phones vs. tablets? I guess there it would make sense to block all devices.

Confusing.

@koenpunt
Copy link
Collaborator Author

I've updated the tests that only iPod and iPhone are not supported and neither are Android devices if the useragent string contains mobile.

pfiller added a commit that referenced this pull request Jul 24, 2013
Disable Chosen on iPhone / iPod and Android mobile devices by default
@pfiller pfiller merged commit 6638dd7 into harvesthq:master Jul 24, 2013
@koenpunt koenpunt deleted the browser-support branch July 25, 2013 07:59
pfiller added a commit that referenced this pull request Jul 26, 2013
- CSS Updates #1368, #1371
- Prevent text selection on Chosen #1374
- single_backstroke_delete defaults to true #1375
- No longer expose classes to window context #1389
- Single select value fix #1392
- Remove javascript:void and attr references #1385, #1377
- Disable Chosen on iPhone / iPod and Android mobile devices
  by default #1388
- Add Destroy Method #1396
- Option to Hide Selected & Disabled Options #1404
- Always clear result highlight #1407
- Replace characters only once #1411

- Document all the things. #1366, #1376, #1383
- Add Gemfile for compass dependency #1373
- Update copyright and license link #1397
- Add @koenpunt as a maintainer #1399
- Add anchors to documentation headers #1395
- Updated stackoverflow instructions #1403
@ursbraem
Copy link

Hmmm... it's not an even trade, as on the iPhone, there's no typeahead for the select. If you're using chosen for a list of 3500 places (like me), there's going to be a lot of swiping.

@pfiller
Copy link
Contributor

pfiller commented Jul 29, 2013

It's easy enough to undo this change in your own project if you feel Chosen is better (just remove the test). This change is about making Chosen's default settings the best they can be.

@stof
Copy link
Collaborator

stof commented Jul 29, 2013

@pfiller then maybe it should be an option (defaulting to disable on mobile) instead of forcing to edit the source and recompile it ?

@ursbraem
Copy link

As well as

AbstractChosen.default_multiple_text = "Select Some Options";
AbstractChosen.default_single_text = "Select an Option";
AbstractChosen.default_no_result_text = "No results match";

which would be nice to be able to pass in as an option (the tool I'm using can't write the data-placeholder) for multi-language use.

But of course hacking the plugin is an option, just normally developers discourage that :-)

@ursbraem
Copy link

I've disabled the test for mobile in my setup. For me, that's better, thanks

@MrBigDog2U
Copy link

The Chosen UI doesn't behave correctly on iPad devices. It just shows the list of options below the Chosen control and doesn't allow the user to select one. I tested this on iPad using both Chrome and Safari and got the same behavior on both.

I recommend adding the iPad to the detection code for unsupported browsers.

@janseliger
Copy link

"It's easy enough to undo this change in your own project if you feel Chosen is better (just remove the test). This change is about making Chosen's default settings the best they can be." by pfiller

Could you please add the browser check as a constuctor option. In my case chosen works like a charm on the static disabled devices.

@koenpunt
Copy link
Collaborator Author

koenpunt commented May 7, 2014

You can alter the display of the select's on mobile by just using CSS.
Example here: http://bavotasan.com/2011/style-select-box-using-only-css/

And the last example would look like this on the iPhone:
screen shot 2014-05-07 at 14 35 55

@sebszocinski
Copy link

Yes thats nice but then you still get the default UI select drop down - ugly and too obtrusive on mobile... I prefer a div like in this site im working on...

image

@kumar74g
Copy link

Has anyone been able to use Chosen on mobile devices?

@MichaelWatzke
Copy link

I tried to activate chosen(only basic single dropdown) on iPad/iPhone/Android and everything seems to be okay.

@milengg
Copy link

milengg commented Jul 22, 2014

chosenongalaxys3

We use Chosen with good results on our mobile web views. The only thing the user needs to do is scroll up slightly after search start. It will be better if Chosen calculate distance to top/bottom and opens where there is more space.

@Rikaelus
Copy link

I agree an option would be nice. I've hacked the code to make the test fail, thereby getting Chosen to work on mobile, but that's obviously not ideal since it'll be overwritten if/when I update Chosen to any newer version.

At the very least, if it's the search field that is believed to present visual problems on mobile, maybe have the test only apply when search is enabled. But a full option would still be best.

@rafenden
Copy link

There is any way to turn on support for iOS and Android devices without hacking chosen lib?

@Ruxton
Copy link

Ruxton commented Jul 30, 2014

@rafalenden you can override the function. Provided you override it before you initialise chosen it seems to work ok.

AbstractChosen.browser_is_supported = function() { return true; } // or replace with your own tests to return false/true

@koenpunt
Copy link
Collaborator Author

@Ruxton the AbstractChosen class is not exposed in the jQuery version.

@rafenden
Copy link

I'm using jQuery version.

@campbeln
Copy link

campbeln commented Sep 3, 2014

Rather than redefining AbstractChosen.browser_is_supported why not just change the two test lines from...
if (!AbstractChosen.browser_is_supported()) {
to...
if (!AbstractChosen.browser_is_supported() && !options.disable_search) {

Then devs who want to have a consistent UI experience (and have multiple selected elements shown nicely on-screen) without the weirdness of the on-screen keyboard can have it.

Of course, this only works if search is disabled, so a valid argument could be made for an options.disable_on_mobile or likewise.

Anyway... my 2c.

gavinhungry added a commit to gavinhungry/sandbug that referenced this pull request Sep 5, 2014
@rafenden
Copy link

I've created fork that works on mobile devices.
Feel free to use it: https://github.com/rafalenden/chosen

@marc-gist
Copy link

Seems to work fine on Android 4.4 ... will test in iPad etc.

so i agree with above people we should get an easy to set option to override this behavour.

@manmohan-bisht
Copy link

Hi Marc..is this working for you on all mobile and touch devices ? If yes could you pls share your... code here..

@marc-gist
Copy link

manmohan-bisht: i just followed what others said to do; on line 504 I did this to the function there

AbstractChosen.browser_is_supported = function() {
      //if (window.navigator.appName === "Microsoft Internet Explorer") {
      //  return document.documentMode >= 8;
      //}
      //if (/iP(od|hone)/i.test(window.navigator.userAgent)) {
      //  return false;
      //}
      //if (/Android/i.test(window.navigator.userAgent)) {
      //  if (/Mobile/i.test(window.navigator.userAgent)) {
      //    return false;
      //  }
      //}
      return true;
    };

    AbstractChosen.default_multiple_text = "Select Some Options";

    AbstractChosen.default_single_text = "Select an Option";

    AbstractChosen.default_no_result_text = "No results match";

    return AbstractChosen;

  })();

I've tested on iPad 2 (fully updated, not sure what version that means), and Android 4.2+ and it seems to work just fine. Can scroll a bit "funny" but works well enough.

@fritzgithub
Copy link

I agree with rockethouse (may 7 2014)
I use Chosen for design and UI but i do not at all need the search feature.
I cannot understand why this cannot be turned off for multiple selects (or do i keep overseeing things?).
Without search (and keyboard opening) on mobile devices, chosen would work like a charm (at least on my single test with firefox an android tablet).
Is there any chance i can deactivate search on multiple selects?

@Mushr0000m
Copy link

Hi, any plans to make Chosen stable on iOS and Android ?

@koenpeters
Copy link

If support for Chosen is eventually added I think it would be a good idea to simply use a full screen view (filter + list) for the phone and small tablet use cases. This mimics the native combo boxes and offers the best usability for users, especially a big chunk of the screen is taken up by a keyboard.

@koenpunt
Copy link
Collaborator Author

koenpunt commented Oct 2, 2015

@koenpeters I like that thought! But probably required a LOT of CSS updates.

@koenpunt
Copy link
Collaborator Author

koenpunt commented Oct 2, 2015

@mlettini maybe you can do a suggestion on what @koenpeters idea could look like?

@mlettini
Copy link

mlettini commented Oct 2, 2015

@koenpunt @koenpeters That is a neat idea, and it might be doable, but it probably requires JS changes too. Because if we're talking CSS-only then you'd need to move the Chosen select out of it's position, which would require position: fixed. And if it's fixed, that usually doesn't work too well on mobile. And then the CSS would need to accommodate if the keyboard is open or not, and I'm not entirely sure how well that is handled (probably "not well at all"). And on top of all this, there's the different mobile OSes and their styles, and then we'd have to support certain mobile browsers... high effort IMO.

If I had the time, I might try a prototype, but alas, I don't right now.

@koenpunt
Copy link
Collaborator Author

koenpunt commented Oct 2, 2015

Issues with position: fixed was the first thing that came to mind for me as well, but if you take a look here: http://caniuse.com/#feat=css-fixed, you'll see that's probably not much of a problem anymore.

@tjschuck
Copy link
Member

tjschuck commented Oct 2, 2015

@koenpeters @koenpunt @mlettini Any additional discussion about enabling Chosen on mobile should probably be a new issue. You can link back to this thread, but this thread is "closed" (via merge), so it's more likely to get lost when people are looking for open feature issues.

@koenpeters Do you mind opening a new issue summarizing from your comment down?

@harvesthq harvesthq locked and limited conversation to collaborators Oct 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.