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

Add back eventHandler arguments in V2 #498

Closed
lougreenwood opened this issue Oct 21, 2019 · 4 comments · Fixed by #499
Closed

Add back eventHandler arguments in V2 #498

lougreenwood opened this issue Oct 21, 2019 · 4 comments · Fixed by #499

Comments

@lougreenwood
Copy link
Contributor

lougreenwood commented Oct 21, 2019

V2 removed the various event handler arguments (onMouseEnter, onFocusOut etc) in favour of the {{on ...}} element modifier.

This was to better align the add-on with Octane patterns.

However, if an end-user extends ember-basic-dropdown to add extra default behaviour for some internal dropdown component, there is a problem in so far as that element modifiers are not usable with the (component) helper and the extending component must yield out the Content & Trigger components, in the same was as ember-basic-dropdown does: https://github.com/cibernox/ember-basic-dropdown/blob/master/addon/templates/components/basic-dropdown.hbs

The problem is further excaberated by attempting to upgrade to Ember 3.13, where ember-basic-dropdown is required because of various deprecations in Ember.

This particular issue seems to be one that is being addressed via an RFC (emberjs/rfcs#497), but until then, this leaves end users such as myself stuck not being able to upgrade to Ember 3.13.

As such, I'd like to propose adding back the arguments for the various event handlers, which can be passed through to various {{on ...}} modifiers on Content & `Trigger, something like:

templates/components/basic-dropdown-content.hbs

<Element
  id={{this.dropdownId}}
  class="ember-basic-dropdown-content ember-basic-dropdown-content--{{@hPosition}} ember-basic-dropdown-content--{{@vPosition}} {{this.animationClass}}{{if @renderInPlace " ember-basic-dropdown-content--in-place"}} {{@defaultClass}}"
  ...
  {{on 'focusout' (optional @onFocusOut}}
  {{on 'mouseleave' (optional @onMouseLeave}}
>
  {{yield}}
</Element>

This could temporarily be added back in ember-basic-dropdown V2 and later removed again in an ember-basic-dropdown V3 release when the previously mentioned RFC is landed in Ember.

Happy to help out with the implementation if you agree! 👍

@cibernox
Copy link
Owner

I can certainly consider it, however I'd like to better understand how are you using the (component) keyword. I believe that there should be a way today of passing element modifiers if you combine it with {{let}}.

@lougreenwood
Copy link
Contributor Author

Yeah, I was discussing this on Discord today and the {{let}} workaround was mentioned, however, because our component essentially extends the ember-basic-dropdown component, the template must yield out an instance of the Content & Trigger components, essentially copying the ember-basic-dropdown.hbs template - here's an overview of our use case:

components/basic-dropdown-styled.js

import EmberBasicDropdown from 'ember-basic-dropdown/components/basic-dropdown';
import layout from 'templates/components/basic-dropdown-styled';

export default EmberBasicDropdown.extend({
  //Custom component default overrides, eventHandlers etc
})

templates/components/basic-dropdown-styled.hbs

{{yield (hash
  uniqueId=this.publicAPI.uniqueId
  isOpen=this.publicAPI.isOpen
  disabled=this.publicAPI.disabled
  actions=this.publicAPI.actions
  Trigger=(component "basic-dropdown-trigger"
    dropdown=(readonly this.publicAPI)
    hPosition=(readonly this.hPosition)
    renderInPlace=(readonly this.renderInPlace)
    vPosition=(readonly this.vPosition)

    onMouseDown=(action this._prevent)
    onMouseEnter=(action this._open)
    onTouchEnd=(action this._open)
    onMouseLeave=(action this._close)
  )
  Content=(component "basic-dropdown-content"
    dropdown=(readonly this.publicAPI)
    hPosition=(readonly this.hPosition)
    renderInPlace=(readonly this.renderInPlace)
    vPosition=(readonly this.vPosition)
    destination=(readonly this.destination)
    rootEventType=(readonly this.rootEventType)
    top=(readonly this.top)
    left=(readonly this.left)
    right=(readonly this.right)
    width=(readonly this.width)
    height=(readonly this.height)

    onMouseEnter=(action this._open)
    onMouseLeave=(action this._close)
    onTouchEnd=(action this._open)
    onFocusOut=(action this._test)
  )
)}}

(Notice the added event handlers in the above)

Which allows us to have the same usage as ember-basic-dropdown, but with a default set of event handler behaviour defined for our component:

<BasicDropdownStyled as |dd|>
  <dd.Trigger>Button here</dd.Trigger>
  <dd.Content>Content here</dd.Content>
</BasicDropdownStyled>

@cibernox
Copy link
Owner

Yeah, I see. Indeed, element modifiers today have to be passed in during invocation. It makes sense to add back the onXXX event handlers for that situation for now, I agree.

@lougreenwood
Copy link
Contributor Author

Cool, I'll likely take a first pass at this tomorrow and submit a PR 👍

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

Successfully merging a pull request may close this issue.

2 participants