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

Allow specifying arbitrary event handlers to any ReactWrapperComponent #39

Merged
merged 2 commits into from
Dec 1, 2018

Conversation

bengry
Copy link
Contributor

@bengry bengry commented Nov 19, 2018

Using the geteventlisteners package to allow capturing arbitrary event handlers specified as @Outputs on any React-wrapper component. e.g.:

<fab-icon iconName="Add" (onClick)="handleIconClick($event)" (onMouseOver)="handleIconMouseOver($event)"></fab-icon>
handleIconClick(ev: MouseEvent) {
  console.log('icon clicked!', ev);  
}

handleIconMouseOver(ev: MouseEvent) {
  console.log('icon moused-over!', ev);  
}

Although extending global prototypes (i.e. not-yours) is bad practice, this is what Angular uses to capture events, and this seemed like the only way to get any arbitrary output from the element.
The other option is to handle each specific event, which is a rather long list, and requires further maintenance, when the DOM, React or the component library adds events to listen to.
This is similar in the idea as us passing any arbitrary attribute to the underlying React component.

Copy link
Contributor

@xjerwa xjerwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and everything seems to be working.
Are the event listeners meant to satisfy a specific scenario or just for functional completeness?

@bengry
Copy link
Contributor Author

bengry commented Nov 30, 2018

@xjerwa Both. In React, everything is a prop, which can be anything really - a primitive, object, a function (callback) etc. However, we can categorize them into 5 categories:

  1. [Native] HTML attributes - Since all of them are just strings (or numbers, though I'm not sure how we deal with numbers today), Angular templates allow passing any attribute to the component, and then ReactWrapperComponent passes them down as-is.
  2. React-component defined "input" props, e.g. a DefaultButton's text prop - we create @Inputs for these in the wrapper Angular component and pass them to the React counterpart via the template.
  3. React-component defined "output" props, these are just callbacks. e.g. Checkbox's onSelect prop - we create @Outputfor these in the wrapper Angular component, catch the callback from React, map all arguments to an object and emit() using an EventEmitter, so the user can listen to it.
  4. React render-props - we handle these today using createInputJsxRenderer in ReactWrapperComponent and similar, which map JSX-returning functions to Angular TemplateRef, ComponentRef or a function returning an HTMLElement.
  5. [Native] HTML events - these are defined in @types/react's DOMAttributes. Theoretically, we could define the relevant ones for each component, e.g. onClick for a <button>, onChange for an <input> etc. But since onClick is relevant for every DOM element at the end of the day, we'd need to define it everywhere - and the same goes for other stuff - This is quite a bit of work, and duplicate code throughout the wrapper components. To avoid that, this PR adds a way to specify arbitrary events on any component that extends ReactWrapperComponent, we then check what events are being listened to, and pass the relevant handlers (which all look the same - output wise, since they're native DOM events) down to the React component, which passes it down to the DOM element that React renders.

As mentioned, ideally the same could be done for any trivial input/prop (i.e. #1, #2 above, #3 too, with some additional work to automatically convert function arguments to an object), the main issue so far has been getting the Angular compiler to play along, but I'll need to re-visit that, since I haven't looked at it in quite some time.

…dlers

# Conflicts:
#	apps/demo/src/app/app.component.ts
@bengry bengry merged commit 13ff801 into master Dec 1, 2018
@bengry bengry deleted the arbitrary-event-handlers branch December 1, 2018 18:47
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 this pull request may close these issues.

2 participants