search

visurel / iconify-angular

Angular implementation of Iconify, strongly inspired from Iconify-React.
19 stars 14 forks source link

coerce input properties #15

Open mokipedia opened 3 years ago

mokipedia commented 3 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report
[ ] Performance issue
[X] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

Inputs need to be set as [inline]="true" so boolean properties, numbers and sizes are correctly handled.

Expected behavior

properties should be able to coerce from string attributes

What is the motivation / use case for changing the behavior?

more infos why you should allow / check for this can be found for example here: https://netbasal.com/trust-but-verify-coerce-your-component-inputs-bdb743e8b579

Environment

Angular ships with a coercion package in Angular CDK 5+, so it should be fine for all allowed Angular versions.


Angular version: 5+
https://github.com/angular/components/tree/5.0.x/src/cdk/coercion

If wanted, I can submit a PR

mokipedia commented 3 years ago

It would also be more aligned with React, as React supports this by default: https://github.com/iconify/iconify/tree/master/packages/react

visurel commented 3 years ago

@mokipedia Sure, this sounds like a great addition. Feel free to submit a PR, otherwise I'll take a look at it whenever I get the time. 👍

cyberalien commented 3 years ago

In new version of React component I've moved all common code to reusable package @iconify/core, which is used by React, Vue and Svelte components. New version is rewritten in TypeScript, which makes it easier to export types. See "packages/core" directory.

That should make it easy to update Angular component too. I'd be happy to help if you have any questions, but I'm not familiar with Angular specific stuff.

mokipedia commented 3 years ago

@cyberalien I see what you did there. there is a lot to be shared here. It would also mean quite a bit of refactoring instead of an easy small change like my initial request. I am in favour of refactoring, but currently lack the time to do so without the security of having automated tests.