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 Listeners to multiple components in ViewController #5

Closed
jachenry opened this issue May 1, 2012 · 11 comments
Closed

Add Listeners to multiple components in ViewController #5

jachenry opened this issue May 1, 2012 · 11 comments

Comments

@jachenry
Copy link

jachenry commented May 1, 2012

I have a form that has two submit buttons, one in the upper right hand corner and another at the bottom.

      { 
        xtype: 'button',
        action: 'submit',
        ui: 'action',
        align: 'right'
      },
      ...
      { 
        xtype: 'button',
        action: 'submit',
        ui: 'action'
      },

Currently the only way to add a listener to both the buttons is to give it a unique identifier and create two separate listeners.

    titleSubmitButton: {
      selector: 'button#edit_title_submit',
      listeners: {
        tap: 'onSubmitTap'
      }
    },
    formSubmitButton: {
      selector: 'button#edit_form_submit',
      listeners: {
        tap: 'onSubmitTap'
      }
    },

It'd be nice if there was a way to add a listener to both buttons using a single selector.

    listeners: {
      selector: 'button[action=submit]',
      tap: 'onSubmitTap'
    },
@jachenry
Copy link
Author

jachenry commented May 3, 2012

Below is a selector returning a group of items. Is there an equivalent way to do this inside a DeftJS ViewController?

    ...
    config: {
        control: {
            'searchlist searchlistitem button': {
                tap: 'onSearchDelete'
            }
        }
    },
    ...

@johnyanarella
Copy link
Member

That list item oriented use case is very compelling.

Typically this would be solved by relying on event bubbling and listening to the parent component (the list) rather than creating listeners for each item. However, in your case you're interested in a particular child element within the list item. And, for the moment, you'd have to dispatch a unique event to differentiate between tap events for different buttons within a list item.

I think there's room for improvement here. Thanks for reporting this issue.

@superstructor
Copy link
Contributor

I see two different issues

  1. selecting/listening to multiple components
  2. selecting/listening to components that are created after controller initialization (currently throws an exception)

I've been working on a patch to resolve both.

Issue 1. could be fixed by

  • loop through all matching components binding event listeners
  • change behaviour of component getters like getMyButton to return the first matching component, which should cause minimal breakage to existing API users
  • provide an additional method eachMyButtons that iterates over the matching components yielding each to a function argument (credit to @wombleton for iterator idea)
  • could also provide a getAllMyButtons or similar to return an array but I don't see the point

Issue 2. has two possible solutions.

First possibility is to use event delegation (aka "live events") ala vanilla ExtJS 4 MVC. Can be implemented by overriding Ext.Component.fireEvent to dispatch the event to the appropriate ViewController instance that then executes all component queries live at event firing time rather than at controller instantiation time as it is now. Similarly getters/iterator functions would run queries live.

Second solution is to automatically bind events to new components by listening to 'add' events for all containers controlled by the ViewController instance. When an 'add' event is fired just re-run all component queries against the newly added components and bind events (or create getters etc) for those that match.

The first solution appears to have significantly better performance from my basic testing.

As I don't want to duplicate effort I'm interested to know if you guys are already working on a patch for fixing these issues ?

What approach do you think is best ?

Is it worth me continuing to polish my patch to a state ready for a pull request ? I'll need to add some tests, fix some bugs, refactor it a little etc etc.

I'm enjoying the DeftJS, awesome library =)

Cheers

@superstructor
Copy link
Contributor

Also if there are use-cases where you want to control a component that is not a child of a container from the same ViewController (e.g. a window?) I think an approach like #4 would still be needed in addition to all of the above.

@johnyanarella
Copy link
Member

We're headed toward the 'live events' approach. As you noted, the automatic binding of all matching children based on 'add' is problematic and inefficient. We are leaning toward intercepting fireEvent(). I also agree that we would still want to include an approach for runtime registration and deregistration as in #4.

So far the discussion around this issue and the changes necessary to support 'live events' has delayed resolution of #4 and inclusion of the pull request @wombelton so kindly provided.

I'd definitely be intrigued to see your pull request - we might not use it verbatim but it will certainly influence the outcome. =)

@johnyanarella
Copy link
Member

We are moving forward with the following solution:

  • Introducing a new live option for controlled selectors within a Deft.mvc.ViewController.
    • This option would default to false if omitted, ensuring that the overhead of live event listening is mitigated by making it explicitly an opt-in behavior
  • Implementing a facade that supports both both Ext JS and Sencha Touch (which otherwise internally implement very different event dispatching mechanisms) that exposes methods for listening for live events for a selector relative to a specified Ext.Container.
    • This facade will intercept Ext.Component::fireEvent() and evaluate dispatched events against any registered live event listeners.
  • Modifying Deft.mvc.ViewController automatic getter generation to:
    • produce live-querying getters for controlled selectors that specify true for the live option
    • return a scalar value when there is single match for a selector, and an Array when there are multiple matches
  • Expose public methods for dynamic registration and unregistration as in Allow dynamic component control registration #4.

Special thanks to @superstructor for making time to discuss this issue further and provide feedback and insight into his intended use cases.

@superstructor
Copy link
Contributor

+1, this opt-in approach is a great idea. Thanks for the opportunity to provide feedback!

@johnyanarella
Copy link
Member

I am still concerned about the performance hit associated with intercepting Ext.Component::fireEvent().

I am investigating an alternative implementation that hooks ComponentManager::register() and ComponentManager::unregister() and leverages the Ext.Component added and removed events to detect when a component is added to / removed from a View or a descendent container within in a View, and adding and removing listeners accordingly.

@johnyanarella
Copy link
Member

That approach is proving much more viable. Currently refactoring Deft.mvc.ViewController to accommodate a new live option, live-querying getters, etc.

@johnyanarella
Copy link
Member

The ComponentManager::register() and ComponentManager::unregister() + Ext.Component added and removed events turned out to be the way to go. All of there associated complexity has been encapsulated in Deft.event.LiveEventBus, which provides methods for subscribing and unsubscribing to component events for a view-relative selector. Internally, Deft.event.LiveEventBus will automatically add and remove corresponding event listeners whenever matching components are added to or removed from the view.

Deft.mvc.ViewController was significantly refactored to decompose the representation of registered selectors and listeners into new private Deft.mvc.ComponentSelector and Deft.mvc.ComponentSelectorListener classes.

Deft.mvc.ViewController now supports a live option for selectors specified in the control annotation. This is opt-in as described above - live defaults to false. When true, the Deft.event.LiveEventBus is leveraged to automatically add and remove listeners as components matching the selector are added and removed from the view.

Additionally, automatic getters generated from the control statement now support selectors that match more than one component (regardless of whether the selector is live or not). In those cases, an Array will be returned by the getter.

This new feature is available in the code currently in the master branch and will be shipped as part of Deft JS v0.8.

@johnyanarella johnyanarella reopened this Jul 24, 2012
@johnyanarella
Copy link
Member

Reopening to address newly discovered Sencha Touch compatibility issues.

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

No branches or pull requests

3 participants