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

Proposal: Support declarative binding from View events to Observables #4062

Closed
robwormald opened this issue Sep 8, 2015 · 62 comments
Closed

Proposal: Support declarative binding from View events to Observables #4062

robwormald opened this issue Sep 8, 2015 · 62 comments

Comments

@robwormald
Copy link
Contributor

@robwormald robwormald commented Sep 8, 2015

Not sure this is a good idea, or even possible, but thought I'd throw it out there.

In vanilla Rx, if I wanted to listen to a DOM event to do a typeahead or something I'd do something like:

//get DOM element
const input = document.getElementById('myInput');
//observable of click events
const inputs$ = Rx.Observable.fromEvent(input, 'keyup');
//map changes to requests
const responses$ = inputs$.flatMap(ev => myHttpService.get(ev.target.value)); 
//subscribe to responses
responses$.subscribe(json => console.log(json));

In current ng2, if I wanted to do the equivalent, I'd do something like:

@Component({
  selector: 'my-component'
})
@View({
  template: `
     <div [ng-form-model]="myForm">
       <input type="text" ng-control="query">
    </div>
    <ul>
     <li *ng-for="#result of results | async">{{result.text}}</li>
   </ul>
`
})
class MyComponent {
  constructor(public http: Http, fb: FormBuilder){
    //create form group
    this.myForm = fb.group({
      query: ""
    });
    //subscribe to value changes, map to responses, bind to view w/ pipe
    this.results = this.myForm.controls.query.valueChanges
      .toRx()
      .flatMap(query => this.http.get(`/foos?q=${query}`)
      .map(res => res.json());
  }
}

This all works great (or will, once Rx stuff settles).

However, if I wanted to do the same sort of thing from an arbitrary event (unrelated to forms, like a mousedrag or button click or whatever), there's not a good way (I can see) to get an Observable from a DOM event, so you have to do something like:

<button (click)="makeRequest($event)">Make Request</button>
//snip
class MyComponent {
  makeRequest($event){
    this.http.get($event.target.url)
      .map(res => res.json())
      .subscribe(results => this.results = results);
  }
}

which isn't nearly as clean.

Proposed idea would be to provide a way to delegate / bind an arbitrary DOM event to a Subject/Observable, so that it could be easily subscribed to and handled reactively.

Something like (totally made up syntax here, point being we're binding the click event to the Subject):

Edit: see #4062 (comment) for current proposal

<button [(click)]="clicks$">make request</button>
class MyComponent {
  clicks$ = new Rx.Subject();
  constructor(){
    this.results = this.clicks$
      .flatMap(ev => http.get('someurl.json'))
      .map(res => res.json())
  }
}

I saw #3992, but I believe this is for emitting events "out" from the component, whereas this idea is more for usage within a component.

Thoughts?

@PatrickJS
Copy link
Member

@PatrickJS PatrickJS commented Sep 9, 2015

👍 it would be awesome to have Rx Subjects for two-way data-binding

github-tipe-logo

@dsebastien
Copy link

@dsebastien dsebastien commented Sep 9, 2015

+1, let's RX all the things :)

@jeffbcross
Copy link
Contributor

@jeffbcross jeffbcross commented Oct 29, 2015

I was chatting with @vsavkin earlier today about this, but more from the perspective of putting the component in control instead of the template. He had the idea of extending the @ViewChild property decorator to something like @ObserveChildOrBetterName('.some-button', 'click'),

class MyComponent implements AfterViewInit {
  @ObserveChild('.search-button', 'click') searchClick:EventEmitter<MouseEvent>;

  afterViewInit () {
    this.searchClick.subscribe(evt => alert('clicked'));
  }
}

This could work for native and custom events, and, aside from setting the MouseEvent as the generic type, has no tight coupling to the DOM, so this component is more portable to different platforms.

@ericmartinezr
Copy link
Contributor

@ericmartinezr ericmartinezr commented Oct 30, 2015

@jeffbcross that would be beautiful 👍. Although I would call it @ObserveOutput() to keep it aligned with the latest naming.

@flyingmutant
Copy link

@flyingmutant flyingmutant commented Nov 16, 2015

It would be nice to also use the hypothetical @ObserveChild() to translate this:

@ng.Component({
  template: `<input [ng-form-control]="inputControl">`
})
class Something {
  inputControl = new ng.Control();
  constructor() {
    this.inputControl.valueChanges.subscribe(...)
  }
}

into

@ng.Component({
  template: `<input #my-input>`
})
class Something {
  @ng.ObserveChild('myInput', 'input') inputChanges: EventEmitter<Event>;
  constructor() {
    this.inputChanges.map(ev => ev.target.value).subscribe(...)
  }
}

Having to use [ng-form-control] to observe the values does not feel very natural currently, and clutters the template as well.

@flyingmutant
Copy link

@flyingmutant flyingmutant commented Nov 16, 2015

Aside: isn't it amusing how the things move from onclick-style (attach things right in the DOM), to jquery-style (attach things after the fact using selectors), to angular1-style (= onclick-style reborn), to angular2-style (@ViewChild, @ObserveChild etc. – jquery-style reborn)?

#CleanTemplates2015

@PascalPrecht
Copy link
Contributor

@PascalPrecht PascalPrecht commented Nov 29, 2015

Love @jeffbcross and @vsavkin proposal. Thanks @robwormald for bringing this up.

@ericmartinezr as a site note, I think @ObserveOutput doesn't really fit here, as "outputs" describe what goes out of the component. This pattern however, describes how to listen/subscribe to something inside the component as part of the component's view. So it's not really an output.

But i'm sure we're going to find a nice name here, as naming is sooo easy.

@danculley
Copy link

@danculley danculley commented Dec 22, 2015

Not to add to the naming soup, but I think @ObserveChildEvent() and @ObserveHostEvent() would make the most sense.

@ObserveChild() is not sufficiently generic, as the same concept makes sense for observing DOM events on the host element as well. The semantics also do not match exactly, because the bound variable is observing an event on the child, not the child itself.

@ObserveOutput() is (as @PascalPrecht describes above) confusing because it is not just an observable version of @Output().

@ObserveEvent() identifies the event, but not where it is coming from.

@thelgevold
Copy link
Contributor

@thelgevold thelgevold commented Dec 31, 2015

I have been playing with Observables tied to a UI element and some event.

One issue I've had is passing data via the event - without storing and passing state from the DOM.
I have been using fromEvent, but I end up having to pull info from the DOM element to make it work. The use case is to pass an id to build a dynamic url when an element is clicked.

I would love to be able to just pass arbitrary model data to processes triggered by the event. I would also like to abstract the underlying element and perhaps group it with other elements with the same behavior, but with different data.

One idea I came up with was a concept of an Observable group

<div [ObservableGroup]=['group1',{country:'usa'}]>USA</div>
<div [ObservableGroup]=['group1',{country:'germany'}]>Germany</div>

//Made up function
Observable.fromGroupAndEvent('group1','click')
.switchMap((r:any) => 
    {  
        this.activeCountry = r.data.country;
        return this.http.get('./country-info/' + this.activeCountry + '.json')
    })
.map((res: Response) => res.json())
.subscribe(r => this.capitol = r.capitol);

I am new to Observables, so I am not sure this is the right direction, but it does seem to meet some of my goals

@edispring
Copy link

@edispring edispring commented Jan 18, 2016

👍 for @ng.ObserveChild()

@valorkin
Copy link
Contributor

@valorkin valorkin commented Jan 19, 2016

Observe all the things! 👍

@mhevery
Copy link
Member

@mhevery mhevery commented Jan 27, 2016

I think this is a good idea, post RC.

@timkindberg
Copy link

@timkindberg timkindberg commented Feb 9, 2016

  @ObserveChild('.search-button', 'click') searchClick:EventEmitter<MouseEvent>;

I'm not sure this syntax is consistent with other Angular 2 syntaxes. Regular events, e.g. (click), are controlled from the template. Why would we have this new thing be controlled in the controller?

I'm not saying it's bad overall, but if we are deciding that we no longer want to handle events from the template, then it should be consistent across the board.

We are just looking to sugar this:

<button (click)="clicks$.emit($event)">Make Request</button>
class MyComponent {
  private clicks$ = new Subject();
}

Honestly, I'm not sure sugar is needed, that's already pretty short. I'd also prefer to NOT have another new syntax, let's try to keep the (event) syntax the same.

@e-oz
Copy link

@e-oz e-oz commented Feb 9, 2016

@timkindberg I'm all for brevity and consistency, but selector in proposal will allow to catch events from nodes, not existing in template initially (they could be generated later).

@timkindberg
Copy link

@timkindberg timkindberg commented Feb 10, 2016

@e-oz yes but what stops the regular events (like click) from using a similar syntax as well? Why wouldn't we do normal click events like this then?:

@Event('.search-button', 'click')
onClick($event) {}

I'm just saying it's a bit odd to all of a sudden put the onus on the controller to behave in a way completely unlike anything that's in the API yet. It's setting a very strong precedent that will go against what Angular has always been all about; declare your properties and events in your HTML.

@e-oz
Copy link

@e-oz e-oz commented Feb 10, 2016

@timkindberg it's happened already - @input and @output are key things in angular 2 and they are very handy to use, and simple to read.
About your example and first question: it's again different selector: for list of elements. With (click) in template you can point exact node without any ID or specific class.

@benlesh
Copy link
Contributor

@benlesh benlesh commented Apr 21, 2016

EDIT: Yup, still thinking about the original issue/proposal, no idea we were talking about a different proposal half way through the thread...

Look, I'm clearly all for RxJS'ing the hell out of things. I'm just unsure what the added syntax here buys anyone. Is it that you no longer need to define clicks$ as a property with a Subject on the component? Because having it there provides a hook point for testing, right? Niavely, you can next into that subject any time you want and inspect the outcome. In a more advanced world, you could replace it completely with a "hot observable" from Rx's TestScheduler and run the component through its paces with virtual time and marble diagrams.

@robwormald
Copy link
Contributor Author

@robwormald robwormald commented Apr 21, 2016

This case is easy, as you mentioned: https://plnkr.co/edit/IBKc0O4P0zIAjfbgiokd?p=preview - a simple button that you assign at design-time.

The issue is that doing something like document.querySelectorAll('foo') is generally verboten in angular, as its not platform safe (doesn't work in a web worker, for example).

We have a mechanism for (platform safely) querying the view to return angular-friendly refs:

@Component({
  selector: 'tabs',
  template: `
    <some-child></some-child>
    <ng-content>
      <!-- stuff is projected into here -->
    </ng-content>   
  `
})
class SomeComponent {
  //query the view for all children with a selector
  @ViewChildren('some-child') someChildren:ChildElementType;
  //query the view for a specific child by type
  @ViewChild(SomeChildComponent) someChild: ChildElementType[];
  //query elements projected into the component's ng-content element
  @ContentChildren('tab') tab: Tab;
}

imagine it used like this:

<tabs>
  <tab>Tab 1 content </tab>
  <tab>Tab 2 content </tab>
</tabs>

So in this situation, you can't simply declaratively use (click)="$clicks.next($event)" because the elements don't exist when you define the template (in NG1 terms, they're transcluded) This proposal is effectively an angular-safe equivalent of Observable.fromEvent(document.querySelector('tab'), 'click') - but compatible with custom events defined by Components

@TheLarkInn
Copy link
Member

@TheLarkInn TheLarkInn commented Apr 21, 2016

Not only that but I'd assume we'd want to make it compatible with extensions of EventManagerPlugin that allow you to create custom events like "clickOutsideOfElement" etc.

@benlesh
Copy link
Contributor

@benlesh benlesh commented Apr 21, 2016

[REMOVED] I lacked context

@benlesh
Copy link
Contributor

@benlesh benlesh commented Apr 21, 2016

Apologies, I didn't realize there was two proposals in here. Catching up.

@benlesh
Copy link
Contributor

@benlesh benlesh commented Apr 21, 2016

Okay... feedback: It seems like you'd have to be sure that @ObservableChild('.foo-bar', 'click') was more than just Observable.fromEvent(document.querySelector('.foo-bar'), 'click').

You'll need to do one of two things:

  1. Event delegation to the component it lived on, filtering out the events that matched a particular CSS selector.
  2. Or you can do something during the compilation and processing of content that allows you to identify those elements and set them up in a more direct fashion. I'm sure the latter is probably possible, but I don't know enough about it, and it sounds vastly more complicated than the first option. It may or may not be more efficient than the first option, because you'd be looking at adding N handlers rather than 1 handler, but at least you could remove all handlers that weren't being used.
@robwormald
Copy link
Contributor Author

@robwormald robwormald commented Jul 10, 2016

I sketched out (don't worry about the implementation nor consider using it yet!) what this API might look like in a plunk: https://plnkr.co/edit/bhA0at1BTbFoUzcpJBgs?p=preview

@Component({
  selector: 'my-app',
  providers: [],
  template: `
    <div>
      <h2>Total Count: {{counter | async}}</h2>
      <incrementer></incrementer>
      <button #decrement>decrement</button>
    </div>
  `,
  directives: [Incrementer]
})
export class App {
  //query and listen to component output
  @ObserveChild(Incrementer, 'increment') increments:Observable<any>;
  //query and listen to a DOM element
  @ObserveChild('decrement', 'click') decrements:Observable<any>;
  counter: Observable<number>;

  ngAfterViewInit(){
    this.counter = Observable.merge(
      this.increments,
      this.decrements.mapTo(-1)
    )
    .startWith(0)
    .scan((total, value) => total + value, 0);
  }

}
@PatrickJS
Copy link
Member

@PatrickJS PatrickJS commented Jul 11, 2016

@robwormald when in the component lifecycle will the ObserveChild Observable be available? Right now the prototype is limited to ngOnInit etc. It could be kool if we could get the child Observable within the constructor then we can wire up some crazy stuff.

Also, can we get syntax to "bubble" the event up the Observable child to its parents? I only want to subscribe at the root level so probably something like a passthrough/expose rather than subscribe proxy at each level

github-tipe-logo

@TheLarkInn
Copy link
Member

@TheLarkInn TheLarkInn commented Jul 11, 2016

@gdi2292 oo like am automagic 'controlled' .share() to children?

@PatrickJS
Copy link
Member

@PatrickJS PatrickJS commented Jul 11, 2016

update: the plunker angular version needs to be updated to latest angular for it to work again

I want access to child outputs in the constructor so can have full access to rxjs that way we can unlock the reactive potential out of Angular 2. Here's an example using the initial plunker prototype

screen shot 2016-07-11 at 1 53 11 am https://plnkr.co/edit/8aN2hK?p=preview I was able to get it working on the prototype in a similar hacky way and using `EventEmitter (aka Subject)` as a proxy. With this, we're able to create cycles at any level in the component tree. Now I just want to pass the child Observables up the tree which I guess you could support with by combining `@Output` and `@ObserveChild` as I have in the example and also Observable inputs.

All I want is for everything in constructor to be Observables so we can wire everything up. This includes Input/ViewChild/ViewChildren/ContentChild/ContentChildren (pending router Data/Resolved/Params) which would allow for Angular 2 to become powerful with advanced reactive configs. If anyone wants to deal with sync values they can use the correct lifecycle hooks

@ObserveInput
@ObserveViewChild
@ObserveViewChildren
@ObserveContentChild
@ObserveContentChildren
/
I got all of these working in a prototype via AngularClass/angular2-observe-decorators

github-tipe-logo

@mgechev
Copy link
Member

@mgechev mgechev commented Jul 14, 2016

I like the proposal. The declarative bindings will not only enforce a better style of reactive programming but also stimulate applications with logic-less templates, which will increase cross-platform portability.

On top of that, having access to the Subjects in the controllers' constructors will help us write even more declarative code (as the prototype of @gdi2290 shows).

@fxck
Copy link

@fxck fxck commented Oct 6, 2016

Is this being worked on now that final is out?

@PatrickJS
Copy link
Member

@PatrickJS PatrickJS commented Oct 10, 2016

I have a lib published if someone wants to experiment with the decorators

github-tipe-logo

@fxck
Copy link

@fxck fxck commented Oct 10, 2016

Sure, where can I find it?

@amcdnl
Copy link
Contributor

@amcdnl amcdnl commented Nov 8, 2016

@robwormald @IgorMinar - Is there any status updates on this?

I spoke w/ @gdi2290 the other day and we think I've got a pretty good use case for the issue being discussed.

In angular2-data-table I have the ability for users to re-order table header columns using 2 controls: orderable and draggable. In this scenario, draggable is completely decoupled from orderable. The header container is the orderable and the column cells are draggables. Code.

img

The orderable uses ViewChildren to subscribe to 2 events on each of the draggables like:

@ContentChildren(DraggableDirective, { descendants: true })
private draggables: QueryList<DraggableDirective>;

unfortunately since the column headers that are draggables can change, I have to use Differs to constantly diff the columns, unsubscribe from old one and subscribe to new ones like so:

  @ContentChildren(DraggableDirective, { descendants: true })
  private draggables: QueryList<DraggableDirective>;

  private differ: any;

  constructor(differs: KeyValueDiffers) {
    this.differ = differs.find({}).create(null);
  }

  ngAfterContentInit() {
    this.updateSubscriptions();
    this.draggables.changes.subscribe(
        this.updateSubscriptions.bind(this));
  }

  ngOnDestroy() {
    this.draggables.forEach(d => {
      d.dragStart.unsubscribe();
      d.dragEnd.unsubscribe();
    });
  }

  updateSubscriptions() {
    const diffs = this.differ.diff(this.draggables.toArray());

    if(diffs) {
      const subscribe = ({ currentValue, previousValue }) => {
        unsubscribe({ previousValue });

        if(currentValue) {
          currentValue.dragStart.subscribe(this.onDragStart.bind(this));
          currentValue.dragEnd.subscribe(this.onDragEnd.bind(this));
        }
      };

      const unsubscribe = ({ previousValue }) => {
        if(previousValue) {
          previousValue.dragStart.unsubscribe();
          previousValue.dragEnd.unsubscribe();
        }
      };

      diffs.forEachAddedItem(subscribe.bind(this));
      diffs.forEachChangedItem(subscribe.bind(this));
      diffs.forEachRemovedItem(unsubscribe.bind(this));
    }
  }

using @gdi2290 approach this could be very clean.

@PatrickJS
Copy link
Member

@PatrickJS PatrickJS commented Dec 17, 2016

see current issue for better rxjs support
#13248
github-tipe-logo

@amcdnl
Copy link
Contributor

@amcdnl amcdnl commented Dec 19, 2016

@gdi2290 - should this be closed now?

@mhevery
Copy link
Member

@mhevery mhevery commented Dec 20, 2016

Closed in favor of #13248

@angular-automatic-lock-bot
Copy link

@angular-automatic-lock-bot angular-automatic-lock-bot bot commented Sep 13, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.