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.

Sign up for GitHub

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

prevent bubbling of escape key #2713

Merged
merged 4 commits into from
Oct 11, 2016
Merged

prevent bubbling of escape key #2713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0431f6a
add method to retrieve search field value
koenpunt Oct 2, 2016
926369a
move keydown_checker to AbstractChosen
koenpunt Oct 2, 2016
06ef830
update keyup_checker
koenpunt Oct 2, 2016
b05d09e
prevent event bubbling on escape when results are showing
koenpunt Oct 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 7 additions & 33 deletions coffee/chosen.jquery.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class Chosen extends AbstractChosen
@results_showing = true

@search_field.focus()
@search_field.val @search_field.val()
@search_field.val this.get_search_field_value()

this.winnow_results()
@form_field_jq.trigger("chosen:showing_dropdown", {chosen: this})
Expand Down Expand Up @@ -312,7 +312,7 @@ class Chosen extends AbstractChosen
if this.result_deselect( link[0].getAttribute("data-option-array-index") )
this.show_search_field_default()

this.results_hide() if @is_multiple and this.choices_count() > 0 and @search_field.val().length < 1
this.results_hide() if @is_multiple and this.choices_count() > 0 and this.get_search_field_value().length < 1

link.parents('li').first().remove()

Expand Down Expand Up @@ -402,8 +402,11 @@ class Chosen extends AbstractChosen
@selected_item.find("span").first().after "<abbr class=\"search-choice-close\"></abbr>" unless @selected_item.find("abbr").length
@selected_item.addClass("chosen-single-with-deselect")

get_search_field_value: ->
@search_field.val()

get_search_text: ->
$('<div/>').text($.trim(@search_field.val())).html()
$('<div/>').text($.trim(this.get_search_field_value())).html()

winnow_results_set_highlight: ->
selected_results = if not @is_multiple then @search_results.find(".result-selected.active-result") else []
Expand Down Expand Up @@ -457,35 +460,6 @@ class Chosen extends AbstractChosen
@pending_backstroke.removeClass "search-choice-focus" if @pending_backstroke
@pending_backstroke = null

keydown_checker: (evt) ->
stroke = evt.which ? evt.keyCode
this.search_field_scale()

this.clear_backstroke() if stroke != 8 and this.pending_backstroke

switch stroke
when 8
@backstroke_length = this.search_field.val().length
break
when 9
this.result_select(evt) if this.results_showing and not @is_multiple
@mouse_on_container = false
break
when 13
evt.preventDefault() if this.results_showing
break
when 32
evt.preventDefault() if @disable_search
break
when 38
evt.preventDefault()
this.keyup_arrow()
break
when 40
evt.preventDefault()
this.keydown_arrow()
break

search_field_scale: ->
if @is_multiple
h = 0
Expand All @@ -498,7 +472,7 @@ class Chosen extends AbstractChosen
style_block += style + ":" + @search_field.css(style) + ";"

div = $('<div />', { 'style' : style_block })
div.text @search_field.val()
div.text this.get_search_field_value()
$('body').append div

w = div.width() + 25
Expand Down
42 changes: 8 additions & 34 deletions coffee/chosen.proto.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class @Chosen extends AbstractChosen
@container.addClassName "chosen-container-active"
@active_field = true

@search_field.value = @search_field.value
@search_field.value = this.get_search_field_value()
@search_field.focus()

test_active_click: (evt) ->
Expand Down Expand Up @@ -228,7 +228,7 @@ class @Chosen extends AbstractChosen
@results_showing = true

@search_field.focus()
@search_field.value = @search_field.value
@search_field.value = this.get_search_field_value()

this.winnow_results()
@form_field.fire("chosen:showing_dropdown", {chosen: this})
Expand Down Expand Up @@ -303,7 +303,7 @@ class @Chosen extends AbstractChosen
if this.result_deselect link.readAttribute("rel")
this.show_search_field_default()

this.results_hide() if @is_multiple and this.choices_count() > 0 and @search_field.value.length < 1
this.results_hide() if @is_multiple and this.choices_count() > 0 and this.get_search_field_value().length < 1

link.up('li').remove()

Expand Down Expand Up @@ -392,8 +392,11 @@ class @Chosen extends AbstractChosen
@selected_item.down("span").insert { after: "<abbr class=\"search-choice-close\"></abbr>" } unless @selected_item.down("abbr")
@selected_item.addClassName("chosen-single-with-deselect")

get_search_field_value: ->
@search_field.value

get_search_text: ->
@search_field.value.strip().escapeHTML()
this.get_search_field_value().strip().escapeHTML()

winnow_results_set_highlight: ->
if not @is_multiple
Expand Down Expand Up @@ -452,35 +455,6 @@ class @Chosen extends AbstractChosen
@pending_backstroke.removeClassName("search-choice-focus") if @pending_backstroke
@pending_backstroke = null

keydown_checker: (evt) ->
stroke = evt.which ? evt.keyCode
this.search_field_scale()

this.clear_backstroke() if stroke != 8 and this.pending_backstroke

switch stroke
when 8
@backstroke_length = this.search_field.value.length
break
when 9
this.result_select(evt) if this.results_showing and not @is_multiple
@mouse_on_container = false
break
when 13
evt.preventDefault() if this.results_showing
break
when 32
evt.preventDefault() if @disable_search
break
when 38
evt.preventDefault()
this.keyup_arrow()
break
when 40
evt.preventDefault()
this.keydown_arrow()
break

search_field_scale: ->
if @is_multiple
h = 0
Expand All @@ -492,7 +466,7 @@ class @Chosen extends AbstractChosen
for style in styles
style_block += style + ":" + @search_field.getStyle(style) + ";"

div = new Element('div', { 'style' : style_block }).update(@search_field.value.escapeHTML())
div = new Element('div', { 'style' : style_block }).update(this.get_search_field_value().escapeHTML())
document.body.appendChild(div)

w = Element.measure(div, 'width') + 25
Expand Down
48 changes: 42 additions & 6 deletions coffee/lib/abstract-chosen.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,26 +237,62 @@ class AbstractChosen
evt.preventDefault()
this.results_show() unless @results_showing or @is_disabled

keydown_checker: (evt) ->
stroke = evt.which ? evt.keyCode
this.search_field_scale()

this.clear_backstroke() if stroke != 8 and @pending_backstroke

switch stroke
when 8 # backspace
@backstroke_length = this.get_search_field_value().length
break
Copy link
Member

Choose a reason for hiding this comment

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

Ditto on all of these breaks — can be safely removed.

when 9 # tab
this.result_select(evt) if @results_showing and not @is_multiple
@mouse_on_container = false
break
when 13 # enter
evt.preventDefault() if @results_showing
break
when 27 # escape
evt.preventDefault() if @results_showing
break
when 32 # space
evt.preventDefault() if @disable_search
break
when 38 # up arrow
evt.preventDefault()
this.keyup_arrow()
break
when 40 # down arrow
evt.preventDefault()
this.keydown_arrow()
break

keyup_checker: (evt) ->
stroke = evt.which ? evt.keyCode
this.search_field_scale()

switch stroke
when 8
when 8 # backspace
if @is_multiple and @backstroke_length < 1 and this.choices_count() > 0
this.keydown_backstroke()
else if not @pending_backstroke
this.result_clear_highlight()
this.results_search()
when 13
break
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to add all of these explicit breaks here; CoffeeScript handles that.

Docs:

Switch statements in JavaScript are a bit awkward. You need to remember to break at the end of every case statement to avoid accidentally falling through to the default case. CoffeeScript prevents accidental fall-through...

Confirmed with the "Try CoffeeScript" dingus on http://coffeescript.org/ as well.

Copy link
Collaborator Author

@koenpunt koenpunt Oct 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I know, but if you don't add them, CoffeeScript returns a value, and I wanted to get rid of the return true below, so for consistency I added it everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Fair enough.

when 13 # enter
evt.preventDefault()
this.result_select(evt) if this.results_showing
when 27
break
when 27 # escape
this.results_hide() if @results_showing
return true
when 9, 38, 40, 16, 91, 17, 18
break
when 9, 16, 17, 18, 38, 40, 91
Copy link
Member

Choose a reason for hiding this comment

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

My hero.

# don't do anything on these keys
else this.results_search()
else
this.results_search()
break

clipboard_event_checker: (evt) ->
setTimeout (=> this.results_search()), 50
Expand Down