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

clip results instead of moving them #2727

Merged
merged 4 commits into from
Oct 21, 2016
Merged

clip results instead of moving them #2727

merged 4 commits into from
Oct 21, 2016

Conversation

koenpunt
Copy link
Collaborator

@koenpunt koenpunt commented Oct 15, 2016

I have tested this in the following browsers:

Windows 7 macOS Sierra
Latest Chrome Latest Chrome
Latest Firefox Latest Firefox
IE8 Latest Safari
IE9

There's only 1 issue on iOS, and that's that the cursor is visible, even when clipped:
image

fixes #1413
fixes #1410
fixes #1686
fixes #2402
fixes #2445
fixes #2185

@koenpunt
Copy link
Collaborator Author

@harvesthq/chosen-developers @stof as this solves quite a few issues, I'd like to see this merged, so please have a look :)

@tjschuck
Copy link
Member

/cc @mlettini because all CSS.

@mlettini
Copy link
Contributor

I forget our iOS stance, but I think it's still "Chosen is not optimal on iOS", so I'm +1 on this.

@koenpunt
Copy link
Collaborator Author

Why do we use the search input for capturing focus? Can't we just add a tabindex to the container?

@kevin-brown
Copy link

Why do we use the search input for capturing focus?

I'm going to place my bets on it involving browsers not properly sending keystrokes when it was sent to the container, so a search input was used so you can properly preserve the keystrokes that people use when focused on the input.

Note that this is why Select2 did the exact same thing before, and the browser situation hasn't improved.

@koenpunt
Copy link
Collaborator Author

Everybody agrees with @mlettini?

Copy link
Contributor

@adunkman adunkman left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me!

@jim-at-miramontes
Copy link

Sorry if I'm muddying things up here, but I'm not quite following the discussion flow. I'm running v. 1.7.0 of chosen, which, after checking the code, seems to have the file updates posted as part of this page, but I'm still seeing the cursor on iPad. Should I be (or not be)? Is the point of the discussion that it's being punted for now, and that a real resolution of the issue will be coming later? Thanks!

@tjschuck
Copy link
Member

@jim-at-miramontes It seems like the PR was to fix the issue on desktop browsers, and the iPad issue was punted on.

sdemjanenko added a commit to meraki/chosen that referenced this pull request Aug 14, 2017
* tag 'v1.7.0': (76 commits)
  Go to 1.7.0
  add max_shown_results spec for prototype
  replace event.simulate for simulant library
  update grunt tasks
  rename _template functions to _html
  move style block to external css file
  move html templates to abstract class
  rewrite search_field_scale to prevent CSP errors
  add Content-Security-Policy to html
  consider disabled fieldset for disabled state (harvesthq#2718)
  only prevent default on touchevents when results are not showing (harvesthq#2725)
  clip results instead of moving them (harvesthq#2727)
  prevent action on clipboard events when disabled (harvesthq#2722)
  prevent activating disabled fields (harvesthq#2721)
  always close on chosen:close event (harvesthq#2711)
  restore focus if chosen was activated (harvesthq#2710)
  Contributing: add grunt command for tests [ci-skip] (harvesthq#2700)
  do not clear search text on multi selection (harvesthq#2709)
  escape default text (harvesthq#2712)
  add white-space: pre for search field scaling (harvesthq#2714)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment