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

Jump to bottom

Don't use jQuery events as proxies for DOM Events. #3108

Closed
Parakleta opened this issue Sep 29, 2015 · 6 comments
Closed

Don't use jQuery events as proxies for DOM Events. #3108

Parakleta opened this issue Sep 29, 2015 · 6 comments
Labels
Confirmed Bug High Priority
Milestone
2.1.5

Comments

@Parakleta
Copy link 

node.dispatchEvent(new Event(name, {bubbles: true}));

should be used in place of

$node.trigger(name);

since the former is available to all listeners whereas the latter is only available to jQuery or to listeners attached directly to $node (i.e. it bubbles in jQuery but not in DOM).

In the case of the blur event the node.blur() function can be used instead.

The relevant sections of code are:

checkbox.js:507
dropdown.js:2187
search.js:213

What led me to this was trying to get <select> dropdowns to work with React. Without this modification the settings.onChange function must be set to update the React state. With this modification the following examples all function the same way:

  • Without Semantic UI:
    <select onChange={this._change}>
        <option value=1>Value 1</option>
        <option value=2>Value 2</option>
    </select>
  • With Semantic UI CSS (no JS):
    <select onChange={this._change} className="ui dropdown">
        <option value=1>Value 1</option>
        <option value=2>Value 2</option>
    </select>
  • With Semantic UI CSS + JS:
    <select onChange={this._change} className="ui dropdown">
        <option value=1>Value 1</option>
        <option value=2>Value 2</option>
    </select>

    $('select.dropdown').dropdown();

This makes the integration with React automatic in these cases.
@Parakleta
Copy link
Author

Reviewing the React code further, in the case of checkbox.js:507 react requires a click event which can be generated with node.click(). The comments in the code for React claim that the change event is unreliable in IE, and so they ignore it.

@jlukic
Copy link
Member

jlukic commented Sep 29, 2015

Yeah, I always assumed trigger was using native events. Very surprised that they are not.
http://jsfiddle.net/f40wLvuw/

Marked as a bug

@jlukic jlukic added this to the 2.1.5 milestone Sep 29, 2015
@jlukic jlukic modified the milestones: 2.1.6, 2.1.5 Nov 1, 2015
jlukic added a commit that referenced this issue Nov 1, 2015
@jlukic
#3108 Adds native change to dropdown
03a0d87
jlukic added a commit that referenced this issue Nov 1, 2015
@jlukic
Swaps out all calls to trigger() #3108
e5aca0a
@jlukic
Copy link
Member

jlukic commented Nov 1, 2015

Added in 2.1.5

@jlukic jlukic closed this as completed Nov 1, 2015
jlukic added a commit that referenced this issue Nov 1, 2015
@jlukic
#3108 Adds native change to dropdown
528a510
jlukic added a commit that referenced this issue Nov 1, 2015
@jlukic
Swaps out all calls to trigger() #3108
2bae116
@trigoesrodrigo
Copy link

This change caused Backbone Views not to trigger events (Backbone uses jQuery under the hood to listen for events).

Before I was able to declare the change event on selects converted to dropdowns with SemanticUI, now the events are not intercepted.

Does anybody have a clue why this is happening to me, or how to circumvent this problem?

See fiddle with 2.1.5 and change events not being intercepted by Backbone http://jsfiddle.net/trigoesrodrigo/yko902w0/
See fiddle with 2.1.3 and change events being incercepted correctly by Backbone http://jsfiddle.net/xy95h6ej/1/

@Parakleta
Copy link
Author

I've just looked into this, the issue is that the changes have created the events with the wrong bubble/cancel flags. They should be reversed. Standard events should be bubbling and non-cancelable, but the changes added here have created non-bubbling cancelable events instead.

@jlukic You need to swap the booleans on your initEvent calls.

See initEvent and change event documentation.

Bubbling is required for a lot of other libraries because they install a central event handler which needs to catch and filter the events once they hit the top level.

@jlukic
Copy link
Member

jlukic commented Nov 7, 2015

This was patched, in 2.1.6, this was a typo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Confirmed Bug High Priority
Projects
None yet
Milestone
2.1.5
Development

No branches or pull requests
3 participants
@jlukic @trigoesrodrigo @Parakleta