Adding a new function

[Amrrx]

Inject an attribute value to the HTMLElement (Raw attributes) without changing anything else in attr string

[nonara]

Hi @Amrrx. Just checking up on this. Were you planning to add the tests so we can merge this or would you like me to close it?

[Amrrx]

HI @nonara. Thank's for following up. Yeah I was thinking about what kind of tests that may cover such a function and then I got totally busy but i will do it for sure. I also appreciate any kind of support for test cases ideas to implement.

[nonara]

I appreciate the response! I had a closer look into the function. I'm not sure I'm understanding what the use case would be for this.

Can you help me understand what problem this is solving?

[Amrrx]

The main idea is to inject any new attributes without affecting the current attributes string object, so am using the main raw attribute object string to concatenate my new attribute key and value. To better understand why am doing it. Just try to insert a new attribute in something like the following

<app-auto-complete (dataChange)="systemSelected($event)" [url]="'/TechniqueSystem/GetTechniqueSystemDropDown'"
        formControlName="techniqueSystemId" [isRequired]="true"
        [placeholder]="'brandServerPort.selectSystem' | translate" [isMultiple]="false"
        [itemName]="'brandServerPort.system' | translate">
      </app-auto-complete>

Using the normal function, you will notice that the old attributes get confused and became useless, But applying the same with the raw injection, you will get the same attributes string object as its in addition of your injected key value.

[nonara]

Thank you for the explanation! I think it would be better to fix the underlying issue and make sure setting attributes works properly, regardless the attributes format.

If you'd like to try that fix, I'll gladly review. If not, if you'd like to add an issue with the explanation you gave above, I will look into fixing the issue.

[Amrrx]

Well, I do believe the best fix is a separate function as some people do not consider the current behavior as an issue. The current behavior is converting all keys to lowercase and applying some string manipulation into the object which may be useful to some cases but not to mine. So I encourage you to think about it as an extra function that server a different purpose not as a bug that we need to fix.

[nonara]

the old attributes get confused and became useless

I took this to mean that it was actually breaking the attributes.

The current behavior is converting all keys to lowercase and applying some string manipulation into the object which may be useful to some cases but not to mine

Thanks for clarifying. The attributes logic is honestly a bit messy, IMO. I hope to refactor that at some point into something more coherent and DRY. I'm not sure lowercase key conversion should be happening at all in terms of overwriting the rawAttrs property. Lowercase conversion is already performed in the attrs getter, and it's specifically not performed for the attributes getter. Like I said — a bit of a mess. I'll have to look at it more closely later.

In any case, I think the best route for now is to just make rawAttrs public.

In general, I don't like the idea of cluttering up the API even more to add another near-identical method in order to compensate for less-than-ideal behaviour elsewhere in the library. I imagine that that approach is what got us where we are. Right now, the properties and methods surrounding attributes are excessively frayed. I'd like to get them unified and trimmed down to something that's more MDN compliant in the future. (If I do, it will be a major release and existing properties / methods would be soft-deprecated)

Anyway, just to be clear, none of that is a criticism of your work; rather, it's just an observation on the current state of that facet of the library. The route you chose was reasonable, I'd just prefer we work in view of a future cleanup.

If you'd like to go ahead and revert these changes and make rawAttrs a public property, I'll merge. No tests will be needed for that!

[Amrrx]

@nonara I've reverted my own changes to its original and i exposed the rawAttrs object to public.

[nonara]

Great. Thanks for your patience and the contribution!

Looks like we can drop the .gitignore line. package-lock.json is already in there. Not sure why, but it's preventing from merging due to conflicts. May want to double check and make sure it's caught up with main.

[Amrrx]

Okay, I believe that everything is fine now.

[nonara]

Released in v5.1.0