-
Notifications
You must be signed in to change notification settings - Fork 1
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
WebComponents Feedback #2
Comments
Just to clarify, you would recommend creating a new package called In this package would simply be a module that handles the imports and registrations automatically. So, when a developer imports the module into their application, all the heavy lifting is done automatically. I like this idea, and is similar to what we do for React. I do need clarification on one thing you mentioned. You said "if you abstract each web component inside of an Angular component". Do you mean create a native angular component to wrap the existing web components and then duplicate all the properties/methods and bind them to the existing web components? Or is there a different approach you know of? |
Yes, I think creating a separate package for the Angular library would make the setup straightforward. This library would depend on
Yes, exactly that. I don't know of a different approach than duplicating the inputs/outputs. Even when migrating from Angular.js to Angular with the code the Angular team created, us end-users had to duplicate them. From here: |
I'm concerned about the maintainability of duplicating the properties and methods. Especially since the RevealView has some nuisances about how certain features are enabled by setting callback functions to properties. It would add quite a bit to my maintenance overhead. Since web components do work natively in Angular, maybe if we improve the guidance around using the web components, it would be a nice balance between maintaining the library and making it easy to use. Would shipping the Thougths? |
Actually only the declarations of inputs and outputs need to be duplicated, not the methods.
For end-users to use |
If I had a method/function on the web component called |
Also, one more question... If we did a native angular wrapper which copies the inputs/outputs, what do you think the name of the selector should be? Since we cannot reuse |
I created a branch where I can play with ideas and possibly support your request. Feel free to review and give your thoughts. I am not an angular dev, but I know enough to be dangerous 😃 |
Yes, that's right. One option could be, instead of exposing every method that allows controlling the visualization, to expose a single "getController()" method that returns a "controller" class that has all the methods like Some others like
Some options that could work:
|
Thinkering a bit about what this wrapper could look like: Reveal-side codereveal.module.tsexport abstract class RevealInitializer {
abstract configure(): Promise<void> | void;
}
export const REVEAL_INITIALIZER = new InjectionToken<RevealInitializer>("RevealInitializer");
@NgModule({
imports: [CommonModule],
exports: [RvaRevealViewComponent],
declarations: [RvaRevealViewComponent],
providers: [RevealService],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
})
export class RevealModule {
} reveal.service.ts@Injectable()
export class RevealService {
constructor(@Optional() @Inject(REVEAL_INITIALIZER) private revealInitializer: RevealInitializer) {
}
public afterInitialization(): Promise<void> {
return import("./reveal-deps")
.then(result => result.revealLoadedPromise)
.then(() => this.revealInitializer && this.revealInitializer.configure());
}
} rva-reveal-view.component.tsimport { Component, Input } from "@angular/core";
@Component({
selector: "rva-reveal-view",
template: `
<rv-reveal-view [dashboard]="dashboard"/>
`,
})
export class RvaRevealViewComponent {
@Input() dashboard: any;
} This way, a customer can use Reveal like this: Customer-side codemy-lazy-loaded.module.ts@NgModule({
imports: [
RevealModule,
...
],
declarations: [
MyBusinessThingComponent,
],
providers: [
{ provide: REVEAL_INITIALIZER, useClass: MyRevealInitializer },
],
})
export class MyLazyLoadedModule {
} my-reveal-initializer.tsclass MyRevealInitializer extends RevealInitializer {
configure() {
RevealApi.RevealSdkSettings.setBaseUrl(SOMETHING_HERE);
RevealApi.RevealSdkSettings.requestWithCredentialsFlag = true;
RevealApi.RevealSdkSettings.setAdditionalHeadersProvider(SOMETHING_HERE);
defineRevealSdkWrappers();
}
} my-business-thing.component.ts@Component({
selector: "my-business-thing",
template: `
<rva-reveal-view *ngIf="rvdashboard" [dashboard]="rvdashboard"/>
`,
})
export class MyBusinessComponent implements OnInit {
rvdashboard: any;
constructor(private revealInitializerService: RevealInitializerService,
...) {
}
ngOnInit() {
this.revealInitializerService
.afterInitialization()
.then(() => DO_BUSINESS_STUFF)
.then(dashboardData => {
$.ig.RVDashboard.loadDashboardFromContainer(dashboardData,
revealDashboard => this.rvdashboard = revealDashboard,
error => ...);
});
}
} Further explanation of this implementation:
Finally, a possible implementation of reveal-deps is as follows: Reveal-side codereveal-deps.tsimport dayjs from "dayjs/dayjs.min";
(<any>window).dayjs = dayjs;
// Reveal is loaded asynchronously by adding a "script" tag to the body. This code wraps the `<script onload=...>`
// callback into a promise so the rest of our code can know when Reveal has finished loading.
const revealLoadedPromise = new Promise<void>((resolve, reject) => {
const script: HTMLScriptElement = document.createElement("script");
script.src = "assets/reveal/infragistics.reveal-1.7.0.js";
script.type = "text/javascript";
script.onload = () => resolve();
script.onerror = error => reject(error);
document.body.appendChild(script);
});
export { revealLoadedPromise }; |
Something more: for the case of callbacks that are currently inputs and don't return anything, they can be converted to outputs like this (in the Reveal-Angular wrapper): @Component({
selector: "rva-reveal-view",
template: `
<rv-reveal-view [dashboard]="dashboard"
[dataLoading]="dataLoadingFn"/>
`,
})
export class RvaRevealViewComponent {
@Input() dashboard: any;
@Output() dataLoading = new EventEmitter<DataLoadingArgs>();
protected dataLoadingFn = (args: DataLoadingArgs) => this.dataLoading.emit(args);
} This way customers can use it as an output instead of an input (and can use methods instead of functions without dealing with |
I have decided to just duplicate them. At first, I liked the
I like the
Thanks for the detailed examples. The approach I'm talking for these Angular wrappers is not going to be module based, but rather stand-alone components. I also do not want to make this too opinionated or veer too far from the existing jquery based RevealView and web components wrappers. I will leave the loading of the dependencies and the Angular wrapper components to the developer.
The problem with using outputs is that it treats them like they are events, when they are not events. This becomes an issue for a number of the callback properties because if they have a binding (which will happen with The root of this complexity comes from the jQuery based RevealView which does not use events when it should have. I've updated the angular wrapper wiht the remaining properties and functions: https://github.com/RevealBi/reveal-sdk-wrappers/blob/angular/packages/wrappers-angular/src/components/reveal-view/reveal-view.component.ts If this looks good to you, I can publish the initial package and see how it feels for you. |
Was the |
No, it returned |
The rva wrapper looks good to me, I can give it a try once it's in npm |
I published the initial version to npm under the package name It uses standalone components so no module import is needed. Just import the component like Let me know what you think. |
I'm testing the Beta of Web Components, and the documentation instructs me to use
CUSTOM_ELEMENTS_SCHEMA
. Using this is something we wouldn't like to do in our app as that makes Angular's compiler to not warn us about legitimate errors, like a typo in a component's name or a component that is outside of the scope of the current module.Moreover, the documentation says:
Which is not really accurate, it should be added to the module from where the users are going to invoke the Reveal web components. In Angular, each module can choose to use
CUSTOM_ELEMENTS_SCHEMA
, and only in those modules where you allow unknown elements you can use unknown elements (it is not inherited by submodules either).Instead of instructing your users to use
CUSTOM_ELEMENTS_SCHEMA
and thedefineRevealSdkWrappers()
you could encapsulate both in an Angular module, hiding the fact we're using web components as they'll be plain Angular components. This can be done by creating a module calledRevealWebComponentsModule
, which invokesdefineRevealSdkWrappers()
and containsCUSTOM_ELEMENTS_SCHEMA
. Then your users would import that module in any module they want.Doing it this way has a second added benefit: if you abstract each web component inside of an Angular component, you'll get full IDE support: contextual documentation for inputs and components, and detection of wrong input/output names or types.
And another added benefit is it allows users to load only the subset of components they want by importing the desired modules (which is a more Angular way of doing it than invoking a global function).
By the way, there is no need to put the
<script>
tags in the HTML page, that can be lazy-loaded too with:import("./reveal-deps")
from the Angular module.The text was updated successfully, but these errors were encountered: