[ACS-8065][ADF] Unify components in TagModule #10226
Conversation
tamaragruszka
requested review from
eromano,
popovicsandras and
DenysVuika
as code owners
lib/content-services/src/lib/tag/tags-creator/tags-creator.component.ts
Outdated
Show resolved
Hide resolved
| name: 'adfStringsToChips', | ||
| standalone: true | ||
| }) | ||
| export class AdfStringsToChipsPipe implements PipeTransform { |
There was a problem hiding this comment.
we don't need this pipe to be in the core, please just use this code in the component instead
There was a problem hiding this comment.
transforming other way will need much more problematic changes, that was the cleanest way to do it
There was a problem hiding this comment.
you can do the same in the component, what's the problem moving this function there? this pipe does zero performance saving as it's not even marked as pure
There was a problem hiding this comment.
Pipes are pure by default, so no need mark it manually. Also map function is returning brand new array.
In this scenario I am not changing any variable data type, so the change happens while passing data between components. There is already a lot of different variables for different operations.
The only way is to add input field for string array, which will need additional checks on every array change, which will cause additional calls on change.
lib/core/src/lib/dynamic-chip-list/dynamic-chip-list.component.ts
Outdated
Show resolved
Hide resolved
...ervices/src/lib/content-metadata/components/content-metadata/content-metadata.component.scss
Outdated
Show resolved
Hide resolved
|
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/ACS-8065
What is the new behaviour?
all components in tag module use dynamic chip component
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: