-
Notifications
You must be signed in to change notification settings - Fork 51
Angular native wrappers - #3933 #4180
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
Angular native wrappers - #3933 #4180
Conversation
631c41d to
ee291d8
Compare
evenstensberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more comments. Good work here so far!
| @@ -0,0 +1,77 @@ | |||
| /* eslint-disable */ | |||
| /* tslint:disable */ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any way to enable ts/eslint here when the pr is mature enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not that I know of, this is a file generated by the stencil output target so if we change it here it will just add it in again when the package is built next.
There also aren't any other config options for the library to omit generating these lines that I know of.
| defineCustomElementFn(); | ||
| } | ||
|
|
||
| if (inputs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline proxyInputs(cls, inputs);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(if(cond) inline();)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, this is a generated file. With these single line conditionals I would agree with the styling, but again it's not something I know how to or even if it can be controlled.
| import { ValueAccessor } from "./value-accessor"; | ||
|
|
||
| @Directive({ | ||
| /* tslint:disable-next-line:directive-selector */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible to enable lint again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comments
| import { ValueAccessor } from "./value-accessor"; | ||
|
|
||
| @Directive({ | ||
| /* tslint:disable-next-line:directive-selector */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
,--,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comments
| import { ValueAccessor } from "./value-accessor"; | ||
|
|
||
| @Directive({ | ||
| /* tslint:disable-next-line:directive-selector */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
,--,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comments
| import { ValueAccessor } from "./value-accessor"; | ||
|
|
||
| @Directive({ | ||
| /* tslint:disable-next-line:directive-selector */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
,--,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comments
b11e1e9 to
829b728
Compare
743255b to
492581a
Compare
…arget for native components Set up new output targets in the stencil config to output native angular wrapper components in a similar vein to the react components. Versions have been included that will allow the integrating developers to import the components as standalone items one-by-one at need or as a whole module. This also required the addition/alteration of the custom-elements-dist output to create an internal package for the web-components that the angular output plugin could use. The package is included in the pack all script and other checks. Also includes an Angular schematic in the package so users can install simply with "ng add". Documentation has been updated to indicate installation and useage of the new package and components and to provide clarity that this is a community-developed and supported addition as per the ICDS team's request. closes mi6#3933
pushed with no-verify as commitizen was undoing the prettier fixes
…me per PR comment
…i's dependencies Vulnerability recently added for the hono package which is a dependency of a dependency for @angular/cli. Tried updating cli to latest version but this hasn't puhed hono onto a patched version.
…o latest to resolve audit risk
492581a to
c8650ba
Compare
|
This has been moved to a fresh branch: #4206 |
Summary of the changes
Set up new output targets in the stencil config to output native angular wrapper components in a similar vein to the react components. Versions have been included that will allow the integrating developers to import the components as standalone items one-by-one at need or as a whole module. This also required the addition/alteration of the custom-elements-dist output to create an internal package for the web-components that the angular output plugin could use.
The package is included in the pack all script and other checks. Also includes an Angular schematic in the package so users can install simply with "ng add".
Documentation has been updated to indicate installation and useage of the new package and components and to provide clarity that this is a community-developed and supported addition as per the ICDS team's request.
(This is a rebuild of #4054 )
Related issue
closes #3933
& ic-design-system 850
Checklist
General
Testing
Accessibility
Resize/zoom behaviour
System modes
Testing content extremes