Add namespace and make events past tense #1384
Conversation
1b6c74a to
5bca4d0
Compare
|
fix with past tense. |
|
We can support and fire both since adding an 'ed' will not fire the event twice like just adding a namespace does. |
5bca4d0 to
dfe13c2
Compare
|
Updated PR with DEFINITELY breaking changes. The callbacks, Events are now |
| @@ -440,6 +440,13 @@ define(function (require) { | |||
| }); | |||
| }); | |||
|
|
|||
| $('#myPlacard3').on('accept.fu.placard', function() { | |||
There was a problem hiding this comment.
these need to be accepted.fu.placard and cancelled.fu.placard later on down
There was a problem hiding this comment.
Updated. Thanks!
dfe13c2 to
00c0dde
Compare
| @@ -63,30 +63,31 @@ | |||
| constructor: Placard, | |||
|
|
|||
| complete: function (action) { | |||
| var func = this.options['on' + action[0].toUpperCase() + action.substring(1)]; | |||
| var eventName = action === 'accepted' ? 'accept' : 'cancel'; | |||
There was a problem hiding this comment.
var EVENT_MAP = { 'accepted': 'onAccept', 'cancelled': 'onCancel' }; // move up in scope outside function
var func = this.option[EVENT_MAP[action]];
There was a problem hiding this comment.
Sounds good. It allows 'maybe':'imThinkingAboutIt' to be added in the future. j/k
|
Make the suggested adjustment above otherwise the logic for determining a function name is overly complicated. |
00c0dde to
3a7aba9
Compare
|
pushed back to make constants mapper a private variable |
|
Updated. |
Add namespace and make events past tense
This change still needs to be discussed. Fixes #1383.
This is only a breaking change if developers have used
$().on('accept')as 'documented' in the source and not$().on('accept.fu.placard')and$().on('cancel.fu.placard')as the documentation recommends.Also, these appear to be events that did not get switched to past tense.
UPDATED: This is a breaking change for event names of placard.