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.

Sign up for GitHub

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

Jump to bottom

viewportEntered & viewportExited as "real" hooks #7

Closed
rwjblue opened this issue Apr 23, 2015 · 2 comments
Closed

viewportEntered & viewportExited as "real" hooks #7

rwjblue opened this issue Apr 23, 2015 · 2 comments

Comments

@rwjblue
Copy link
Contributor

rwjblue commented Apr 23, 2015

The README uses the term "hook" to refer to boolean values that are set on the root of the component. IMHO, these are "flags" (or something) but a hook would be a function that is invoked.

I would like to have functions that are called instead of forcing observation to consume the mixin. This would make it simpler to do complicated logic on entering/exiting the viewport without relying on using an observer.

We could use the following as a default implementation:

viewportEntered() {
  set(this, 'hasEnteredViewport', true);
  this.trigger('viewportEntered'); // send the `viewportEntered` event
}

Background: I am looking to swap ember-waypoints over to use ember-in-viewport instead of wrapping the jQuery plugin, and I would like to avoid having to use observation for triggering an action.
@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 23, 2015

The proposed change would be a breaking change, so we could change the names of the functions to avoid that breakage if you prefer...

@poteto
Copy link
Collaborator

poteto commented Apr 24, 2015

Working on it!

poteto added a commit that referenced this issue Apr 24, 2015
@poteto
General refactor, fire Evented hooks
d1fffcd 
Closes #7. This commit is a general factor of the Mixin for performance improvements. The Mixin now fires actual 'hooks', named `didEnterViewport` and `didExitViewport`, which you can use in your app in place of observers. For example,

```js
export default Ember.Component.extend(InViewportMixin, {
  handleDidEnterViewport: on('didEnterViewport', function() {
    console.log('entered');
  }),

  handleDidExitViewport: on('didExitViewport', function() {
    console.log('exited');
  })
});
```

Full changelog:

- refactored `_setViewportEntered`
- now triggers Events
- removed unnecessary Ember.warn
- updated debounce method to use fat arrow
- explicitly set `viewportEntered` when removing viewportSpy to prevent race conditions setting it to false
- renamed `_viewportDidEnter` to `_unbindIfEntered` which is more correct semantically
- removed unnecessary var declaration
- removed redundant initial call to `_setViewportEntered`
- removed $viewportCachedEl in favour of using `get(this, 'element')`
@poteto poteto mentioned this issue Apr 24, 2015
Merged
@poteto poteto closed this as completed in #8 Apr 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rwjblue @poteto