Hello,
great work, thanks :)
I think the good practices and conventions section should be in our Coding Conventions[1], what do you think?
[1] https://www.mediawiki.org/wiki/Mobile_web/Coding_conventions
Grüße / Kind regards,
Florian
Von: mobile-l-bounces@lists.wikimedia.org [mailto:mobile-l-bounces@lists.wikimedia.org] Im Auftrag von Joaquin Oltra Hernandez
Gesendet: Mittwoch, 31. Dezember 2014 11:03
An: mobile-l
Cc: mobile-tech
Betreff: [WikimediaMobile] Declarative DOM event delegation in JS views
I'm happy to announce that we've landed on Mantle and MobileFrontend a new way
to specify DOM events on a javascript view that I think will make our views
better in several ways.
TLDR:
* Backbone style declarative delegated DOM events handling is in Mantle's
View class. Use it when possible.
TOC:
* Why. Current approach to binding events
* A different proven approach, delegated declarative DOM events map
* Examples of this in MobileFrontend
* Good practices and conventions
* Gotchas
* Next steps
Why. Current approach to binding events
---------------------------------------
Currently on JS views, when we want to bind some events to the view, we bind
them on the `postRender` method, like so:
var CounterView = View.extend({
// ...
postRender: function() {
// View display logic
this.$('.increment').click(function(ev) {
// ...
})
this.$('.reset').click(function(ev) {
// ...
})
},
// ...
})
With the current approach, a few bad side effects follow:
* Everytime the view renders, we bind events (multiple of them), this is
inefficient, there are better ways.
* The postRender method on views grows a lot in size, event handling
declarations and method code is mixed with display logic.
* The complexity of the view (arguably) increases, and with it the ability to
read and modify the source confidently.
A different proven approach, delegated declarative dom events map
-----------------------------------------------------------------
Inspired and brought from Backbone.js, we have ported their dom events map,
which brings a way of specifying event and selector, and a handler that will be
bound to the view, declaratively on the view itself, the same way we specify
defaults, for example.
http://backbonejs.org/#View-extend
http://backbonejs.org/#View-delegateEvents
The previous example, with this approach with our views would be:
var CounterView = View.extend({
// ...
events: {
'click .increment': 'increment',
'click .reset': 'reset',
},
postRender: function() {
// View display logic
},
increment: function() {
this.counter++;
},
reset: function() {
this.counter=0;
},
// ...
})
On View, we can now specify a `events` entry, which is a map of
`event selector`: handler
Where event is any valid jQuery event, selector is any valid jQuery selector,
and handler is either a function, or a string (name of a method on the view).
With this approach:
* Events are bound with delegate, effectively attaching one DOM listener on
the view's root DOM element. (Read http://api.jquery.com/on/#direct-and-delegated-events)
* To see which DOM events the view binds and reacts to, just need to find the
events map of the view.
* Event handling code (usually pieces of logic of the view) gets a name, and
it's encapsulated on its own method giving us:
* Reuse of such logic/functionality
* Easier and better testability
* Cleaner postRender methods
Internally, this is just using jQuery to delegate the events and the selectors
to specified handlers bound to the view, no black magic.
Examples of this in MobileFrontend
----------------------------------
https://gerrit.wikimedia.org/r/#/c/180835/
https://gerrit.wikimedia.org/r/#/c/180836/
https://gerrit.wikimedia.org/r/#/c/180837/
Notice that the code is much less nested and comes back to View level.
In some cases the postRender method becomes unnecessary, and is deleted, and
the code that was on it, has now a name and is properly encapsulated on the
view.
Good practices and conventions
------------------------------
When creating event handlers, if they are doing DOM stuff, like preventDefault,
or extracting data from the DOM node, manipulating the DOM, etc, you should
name the handler `onEvent`, for example `onThanksClick` or `onHeaderToggle`.
If possible, extract functionality that makes sense independently into methods
with a good name and call them from the handlers as needed.
When creating methods on a view, like the `increment` example above, that
happen to be called when an event happens but make sense independently, I would
argue that a method name `increment` makes more sense than a handler name
`onIncrementClick`, but that is open to common sense and taste I guess.
If I had to also extract what to increment from a DOM element, and then
increment, then I would do both, for example:
// ...
events: {
'click .increment': 'onIncrementClick'
},
onIncrementClick: function(ev) {
var amount = parseFloat(this.$('.amount').val());
this.increment(amount);
},
increment: function(amount) {
this.counter += amount || 1;
},
// ...
Gotchas
-------
Currently, a common practice is inside a event handler to access the DOM
element that initiated the event doing `$(this)`. With the events map the
handlers/methods are at view level and are consistently bound to the view's
this, so how do we access the DOM element of the event?
Handler methods get an jQuery.Event as a parameter, so:
onIncrementClick: function(ev) {
var $incButton = $(ev.target);
// ...
http://api.jquery.com/category/events/event-object/
Next steps
----------
If there is any doubts or concerns, please ask them.
If you find going along the code you spot a View that could benefit of this
approach, open a Phabricator task (prefixed with "Hygiene" and in the
MobileWeb team) and take on it ifyou want.
I think this gives us a good opportunity to get more view tests, and make the
code cleaner, more understandable and approachable for both us and possible
contributors (Backbone style views are known by everybody at this point).