Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#3528 closed enhancement (fixed)

improvement for _trigger-Method and more consistency in slider events

Reported by: trixta Owned by: paul
Priority: minor Milestone: 1.7
Component: ui.slider Version: 1.6rc2
Keywords: Cc:
Blocked by: Blocking:



this is related to ui.core and ui.slider.

I realy like the way ui is triggering events, but i think they could provide more information and consistency in some cases.

I added a diff for both, but will explain the changes here:

  1. The instance-property

In ui 1.5 the all events passed the a reference to the actual ui-instance. I saw that this was changed. But this was a very nice and simple way to get the reference (I know, that i can get the reference in another way). The other problem with changing this, is, that this change could break a lot of existing code. It would be great, if all events would have a subset of same properties (i.e. instance, options). So I changed the line in the _trigger-Method to this: return this.element.triggerHandler(eventName, [e, $.extend({instance: this, options: this.options}, data)], this.options[type]); (I added something similiar to the propagate-Method).

  1. The slide-Event is not consistent (Bug?)

Sometimes the slide-Event is passing two arguments and sometimes it call the event handler with 3 events. This was fixed in the _trigger-Method, but not in the propagate-Method of ui.slider. So i added the following line:

e = e
$.event.fix({ type: n, target: this.element[0] });
  1. noPropagation-parameter in the moveTo-method of ui.slider

I think you have added this feature to break possible circular calling "widgets" (i hope it is clear what i mean.).

I think there is another way to do this. This parameter makes the hole event of the slider-widget unreliable. For example someone makes an extension to the slider an adds his eventlistener. The author of a page needs this extension, but also needs to break this circular calling. But this would break also the extension.

I think it would be much better, if you would pass the parameter to the eventhandler and the eventhandler could then decide what to do. To achieve this, i changed the name of the noPropagation-parameter to callerInfo and this parameter will be passed to the propagate-method:

this._propagate('start', null, {callerInfo: callerInfo});

I know my english is sometimes realy bad. I hope you understand, what i think.

Attachments (2)

core.diff (562 bytes) - added by trixta 7 years ago.
the core.diff
slider.diff (1.6 KB) - added by trixta 7 years ago.
the slider.diff

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by trixta

the core.diff

Changed 7 years ago by trixta

the slider.diff

comment:1 Changed 7 years ago by scottgonzalez

  • Component changed from ui.core to ui.slider

I like the idea of having ._trigger() automatically add in the options hash. However, it was a conscious decision to remove the instance from all events. The average user should not interact with the instance directly. Anything you do with the instance is officially unsupported and may break at any time (we reserve the right to change anything internal at any time).

I've moved this suggestion into a new ticket: #3530. This ticket should now only be relevant to slider. Thanks.

comment:2 Changed 7 years ago by trixta

Anything new on this?

I really think the fact, that the slide-Event is sometimes passing 2 and sometimes is passing 3 arguments to the eventhandler should be treated as a serious bug.

The other issue, switching form the noPropagation-parameter to the the callerInfo-Parameter, would really help to make the slider-events more reliable and more powerfull. Because this thing would mean an api-change, it would be good to come to a decision before the next version (1.6) is out.

comment:3 Changed 7 years ago by paul

  • Milestone changed from TBD to 1.6
  • Resolution set to fixed
  • Status changed from new to closed

Should all fine now after the rewrite.

Note: See TracTickets for help on using tickets.