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.

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

[RFC] Simplifying event listeners binding #22

Closed
titouanmathis opened this issue Jul 6, 2020 · 1 comment
Closed

[RFC] Simplifying event listeners binding #22

titouanmathis opened this issue Jul 6, 2020 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@titouanmathis
Copy link
Contributor

titouanmathis commented Jul 6, 2020

Usage shows us that we often bind multiple event listeners to multiple refs in the mounted hook, and then unbind them in the destroyed hook.

The following proposals aim at reducing this repetitive task to the minimum by providing a logical mapping between refs, events and methods. It would also prevent unwanted memomy leak by systematically removing listeners when a component is destroyed.

Using a strict naming convention for methods

Naming methods based on the ref and event they should be bound:

  • clickHandlerthis.$el.addEventListener('click', this.clickHandler)
  • contentClickHandlerthis.$refs.content[<index>].addEventListener('click', this.contentClickHandler)

The following naming convention should be considered:

  • <refName><EventName>Handler(event[, index]) (the current untold convention)
  • on<RefName><EventName>(event[, index]) (the shortest)
  • handle<RefName><EventName>(event[, index]) (the closest to the event API)
class Foo extends Base {
  get config() {
    return { name: 'Foo' };
  }

  /**
   * Automatically bound/unbound to the `click` event on the `this.$el` element.
   * @param {Event} event The event object.
   */
  clickHandler(event) {}
  // or
  onClick(event) {}
  // or
  handleClick(event) {}

  /** 
   * Automatically bound/unbound to the `click` event on every `this.$refs.content` element.
   * @param {Event}  event The event object.
   * @param {Number} index The current ref index.
   */
  contentClickHandler(event, index) {}
  // or
  onContentClick(event, index) {}
  // or
  handleContentClick(event, index) {}
}

Potential limitations

Adding the same listener to multiple events could become verbose:

class Foo extends Base {
  start() {
    console.log('I am starting!');
  }

  onMousemove(event) {
    this.start();
  } 

  onTouchmove(event) {
    this.start();
  }
}

// Smaller version
class Foo extends Base {
  onMousemove = this.start;
  onTouchmove = this.start;

  this.start(event) {
    console.log('I am starting!');
  }
}

To add multiple listeners to one ref we would have to dispatch them from the method following the naming convention:

class Foo extends Base {
  clickHandler(event) {
    if (this.isOpen) { 
      this.close() 
    }

    if (this.isHovered) { 
      this.highlight() 
    }

    // ...
  }
}

Adding listeners to some ref after another action without the same simplicity could be deceiving:

class Foo extends Base {
  start() {
    console.log('Starting...');
  }

  stop() {
    console.log('Stopping.');
  }
 
  mouseenterHandler() {
    this.start();
    this.$el.addEventListener('mousedown', this.stop, { once: true });
  }
}
@titouanmathis titouanmathis added the enhancement New feature or request label Jul 6, 2020
titouanmathis added a commit that referenced this issue Jul 16, 2020
v1.0.0-alpha.5

The folder tree has been updated for a more robust and more logical structure (32dc7ed):

- `./abstracts/` → does not change
- `./components/` → does not change
- `./services/` → does not change
- `./math/` → moved to `utils/math/`
- `./utils/` → does not change, but its files are grouped by subject in folders : `css`, `math` and `object`

- The Base class `$refs`, `$chilren` and `$options` props are now getters, and event are emitted when they are accessed, following the format `get:...`
- Autbinding of event listeners to refs (df9fc0d, #22):
  - The `onClick(event)` method will be triggered when clicking on `this.$el`
  - The `onFooClick(event, index)` method will be triggered when clicking on any of `this.$refs.foo`
- Refs are now resolved more strictly and their naming convention have been simplified as the component's name is not required anymore:

```html
<!-- Foo.$el -->
<div data-component="Foo">
  <!-- Foo.$refs.bar -->
  <div data-ref="bar"></div>
  <!-- Baz.$el === Foo.$refs.baz -->
  <div data-component="Baz" data-ref="baz">
      <!-- Baz.$refs.bar -->
      <div data-ref="bar"></div>
  </div>
</div>
```

- Add a MediaQuery component (659ebeb)
- Add an Accordion component (dfe2ec5)

- The `nextFrame` function is now async (1fb1fe0):

```js
// Callback usage
nextFrame(() => doSomething());

// Async usage
await nextFrame();
doSomething();
```

- Add CSS utilities
  - `./utils/css/classes.js` exports methods to add, remove or toggle classes
  - `./utils/css/styles.js` exports methods to add or remove styles
  - `./utils/css/transition.js` exports a function to manage CSS transition à la Vue.js (with from, active and to states) (1e89224):

```js
import transition from '@studiometa/js-toolkit/utils/css/transition';

transition(document.body, {
  from: 'opacity-0',
  active: 'transition duration-200',
});
```

- Add and regroup object utilities
  - Move the object utilities to the `./utils/object` folder
  - Replace the `auto-bind` package with a local version for easier reuse
  - Add a `getAllProperties` method to get all the inherited properties of an object

- Fix the pointer service debounced call (d9215b9)
- Fix the pointer service starting state (697a570)
- Rename the EventManager class private properties to follow best practices (db14065)
- Fix the transition for the Modal component (f2e0b79)
- Fix the transition for the Tabs component (8765942)
@titouanmathis
Copy link
Contributor Author

Added in #23 and available in the 1.0.0-alpha.5 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants