Instantsearch.js: Widget events

Created on 11 Sep 2015 · 14Comments · Source: algolia/instantsearch.js

Here's what we could do to be able to have events on widgets. Like refine

app.js

var instantsearch = require('instantsearch');
var search = instantsearch(...);
var refinementList = instantsearch.widgets.refinementList(...);
var slider = instantsearch.widgets.slider(...);

refinementList.on('refine', function(newValue, previousValue) {});
slider.on('slide');
slider.on('change');

search.addWidget(refinementList);
search.addWidget(slider);

Widget.js

var EventEmitter = require('events');
class Widget extends EventEmitter {}

Widget.utils = {
  getContainerNode() {}
}

module.exports = Widget;

IndexSelector.js

var toFactory  = require('to-factory');
var React = require('react');

var Widget = require('../Widget');

class IndexSelector extends Widget {
  getConfiguraition(){}
  init(){}
  render(){}
}

// can use this.emit(); at any time
// can use Widget.utils.getContainerNode of course

// allows to use indexSelector() instead of new `IndexSelector()`
module.exports = toFactory(Widget);

For people to build their own eventful widgets (non events widgets is not an issue here) outside of a browserify/webpack env (where it's easy to extend EventEmitter): we can expose utils in instantsearch namespace like inherits and EventEmitter.

Needs Investigation

Source

vvo

👍1

All 14 comments

What about having addWidget returning the widget itself, so we can write:

search.addWidget(instantsearch.widgets.refinementList(...))
           .on('refine', function(newValue, previousValue) {});

I personally feel it's more intuitive to define configuration and event in the same place where we instanciate the widget.

pixelastic on 14 Sep 2015

@pixelastic :+1: me too

maxiloc on 14 Sep 2015

What about having addWidget returning the widget itself, so we can write:

search.addWidget(instantsearch.widgets.refinementList(...))
.on('refine', function(newValue, previousValue) {});
I personally feel it's more intuitive to define configuration and event in the same place where we instanciate the widget.

I really would prefer the way where instantsearch.widgets.refinementList already send the full eventful widget.

This is my mistake, from the begining I have started documenting addWidget(widgetInstantiation) form and that's not a good pattern at all. I should have documented separated instantatiation and addWidget.

When seeing this:

search.addWidget(instantsearch.widgets.refinementList(...))
           .on('refine', function(newValue, previousValue) {});

As a developer, given my JavaScript level, I would not really understand what it's doing, too much encapsulation and nested parens/method calls.

So some developer would try to rewrite it this way (refactoring, programmatically creating the interface with a configuration) :

var list = instantsearch.widgets.refinementList(...);
search.addWidget(list);
list.on('refine', ..);

This would fail because we only implemented events on the object returned by addWidget: frustration!

Also I must say the second example is easier to reason about, so we might document it this way.

Chainable interface for addWidget might be lying too, given what you propose, why can't I do this:

search.addWidget(instantsearch.widgets.refinementList(...))
           .on('refine', function(newValue, previousValue) {});
           .addWidget(..)
           .on()

Which would not work.

vvo on 14 Sep 2015

What about having addWidget returning the widget itself, so we can write:

So what would be the difference between the objects returned by instantsearch.widgets.refinementList(...) and addWidget, the same object with or without .on on it? Seems like strange and we will get issues about 'on' is not a function on list smg like that.

vvo on 14 Sep 2015

Hmm, I was more thinking of addWidget returning the same object as the one passed. So we can .on on each (because this will be the same object), allowing for both coding styles.

But I agree having too many coding styles is not a good idea, so I'd be ok with the syntax you suggest (define the widget, with the events, then addWidget it), if we update the documentation with it and our code examples.

But maybe the whole search.addWidget(instantsearch.widgets.refinementList()) is too complex, and a search.addRefinementListWidget({...}).on('change') would be better... But let's not go that way for now.

pixelastic on 14 Sep 2015

It would be very disruptive to have addWidget return the same value as the parameter, it would break the chainable pattern expectation. In the chainable pattern the expected return value is the main object itself so you can continue doing more operations on the same object (like in jQuery).

bobylito on 14 Sep 2015

So @proudlygeek has had the need right now. Basically he want to (easily) build an interface where the hits widget would render nothing when the query is empty (1st need), then display a default layout if there's no query.

For this to work he would first need to inform the hits widgets to render nothing when there's no query, and then get an event (maybe just the results event from the helper) to decide to render something else.

This is also what https://jadopado.com/ is doing: when there's an empty query they display a nice homepage, when there's a query they display results.

vvo on 7 Oct 2015

:horse: CLOSING BOT SAYS:

This is very old and not done, reopen if you have the need and want to fix it

vvo on 23 Nov 2015

Found this issue while trying to build out the feature described above in your last comment, @vvo. What is the official solution? I started down the route of triggering custom events from a widget but got stuck when I discovered that doesn't work.