wcm-io / io.wcm.wcm.ui.clientlibs

Extensions for AEM HTML client libraries.
Apache License 2.0
2 stars 0 forks source link

Support custom attributes for CSS and JS includes #2

Closed stefanseifert closed 11 months ago

stefanseifert commented 1 year ago

Fixes #1

Examples:

<sly data-sly-use.clientlib="/apps/wcm-io/wcm/ui/clientlibs/sightly/templates/clientlib.html"
    data-sly-call="${clientlib.css @ categories=['my-clientlib-category'],
    customAttributes=['attr1=value 1','data-attr2=5','attr3']}"/>
<sly data-sly-use.clientlib="/apps/wcm-io/wcm/ui/clientlibs/sightly/templates/clientlib.html"
    data-sly-call="${clientlib.js @ categories=['my-clientlib-category'],
    customAttributes=['attr1=value 1','data-attr2=5','attr3']}"/>
sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

98.2% 98.2% Coverage
0.0% 0.0% Duplication

gouravmakhija18 commented 1 year ago

@stefanseifert - I am curious to know only one question ?.

Why to use "customAttributes" instead to have "categories" same as JSInclude?.

Note: We have predefined attribute for link rel and giving flexibility to have any attribute can pollute clientlibs inclusion in code too ?

e.g. We have only below combinations available.

Case 1: "rel" attribute can have only "prefetch | preload | preconnect | dns-prefetch | prerender | modulepreload" Case 2: "as" attribute can have only "style | script | document | font | image | video | fetch" Case 3: "crossorigin" attribute implementation is already there in JSInclude.java but same is not defined in CSSInclude.java Case 4: Not sure in JSInclude.java & CSSInclude.java file have support for only .css or .js file or other below mention combinations can be possible.

Different "rel" attribute variations

<link rel="prefetch" href="/style.css" />
<link rel="preload" href="/style.css" />
<link rel="preconnect" href="https://example.com" />
<link rel="dns-prefetch" href="https://example.com" />
<link rel="prerender" href="https://example.com/about.html" />
<link rel="modulepreload" href="/script.js" />

Different "as" attribute variations

<link rel="prefetch" href="/articles/" as="document">
<link rel="prefetch" href="/public/app.js" as="script">
<link rel="prefetch" href="/style.css" as="style" />

<link rel="preload" href="/assets/Adobe-Clean.woff2" as="font" type="font/woff2" crossorigin />
<link rel="preload" href="dummy.png" as="image" />
<link rel="preload" href="https://cdn.com/small-file.mp4" as="video" />
<link rel="preload" href="https://cdn.com/file_1.webm" as="fetch" />

Reference link: https://www.debugbear.com/blog/resource-hints-rel-preload-prefetch-preconnect

stefanseifert commented 1 year ago

we currently have a set of HTML Standard attributes supported out of the box, with validation to ensure only valid property values are used, see https://wcm.io/wcm/ui/clientlibs/usage.html

for completely custom attributes like "data-contrast" as listed as example in #1 this PR can help.

maybe there as some HTML standard attributes we currently do not support out of the box we should add support for? the examples you are listing point in this direction - can you create a separate issues for this, that would be a separate PR.

gouravmakhija18 commented 1 year ago

we currently have a set of HTML Standard attributes supported out of the box, with validation to ensure only valid property values are used, see https://wcm.io/wcm/ui/clientlibs/usage.html

for completely custom attributes like "data-contrast" as listed as example in #1 this PR can help.

maybe there as some HTML standard attributes we currently do not support out of the box we should add support for? the examples you are listing point in this direction - can you create a separate issues for this, that would be a separate PR.

Thanks @stefanseifert for the quick response. I have created separate issue for above - https://github.com/wcm-io/io.wcm.wcm.ui.clientlibs/issues/3

sonarcloud[bot] commented 11 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

96.7% 96.7% Coverage
0.0% 0.0% Duplication

henrykuijpers commented 11 months ago

Looks good to me, @stefanseifert !