Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
453 stars 600 forks source link

Add acs-commons- prefixed categories to all clientlibs #617

Closed paul-bjorkstrand closed 8 years ago

paul-bjorkstrand commented 8 years ago

Being an awesome library/tool set this is, it would be great if we could extend the features of AEM Commons with our own enhancements. I suggest adding an acs-commsons- prefixed category to all clientlibs within this package. The use case:

I have a widget I want to extend from the multifieldpanel. Currently, I have two options

  1. Copy/paste all of the ACS Commons code I need to use into another clientlib and modify it there.
  2. Edit the ACS Commons code in place under /apps/acs-commons

Both of these options are bad--if not worst--practice solutions. In clientlibs (at least in the sightly clientlib rendering template/code) there is no way to say "my code needs to depend on acs-commons/touchui-widgets, but should also go into cq.authoring.dialog". Adding a category upon which we can depend on (pun intended) would make this much better, allowing reuse/extension and avoiding the need to modify the ACS Commons provided clientlibs

justinedelson commented 8 years ago

so the issue is that you can't impact the load order of your client library vs. the ACS Commons one?

paul-bjorkstrand commented 8 years ago

That is correct, which makes it impossible to extend widget functionality.

On Mon, Jan 18, 2016, 14:33 justinedelson notifications@github.com wrote:

so the issue is that you can't impact the load order of your client library vs. the ACS Commons one?

— Reply to this email directly or view it on GitHub https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/617#issuecomment-172646713 .

justinedelson commented 8 years ago

ok. take a look at the change I just committed. I think this will serve your purpose. I don't think it is necessary to do this for every single client library, just the widget ones. But let me know if you see other needs.

paul-bjorkstrand commented 8 years ago

Looks good, and I agree it is only necessary for the widget clientlibs.

On Mon, Jan 18, 2016, 18:33 justinedelson notifications@github.com wrote:

ok. take a look at the change I just committed. I think this will serve your purpose. I don't think it is necessary to do this for every single client library, just the widget ones. But let me know if you see other needs.

— Reply to this email directly or view it on GitHub https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/617#issuecomment-172692192 .

mollymehta91 commented 6 years ago

hi @paul-bjorkstrand @justinedelson

I need to tweak following file as per my project requirement "/apps/acs-commons/touchui-widgets/composite-multifield/source/touchui-composite-multifield.js". I created a client library in my project with modified js file. When I embed this js file in acs-aem-commons clientlib, the widget works fine. But, when I embed acs-aem-commons clientlib in my clientlib, the multifield widget doesn't work as expected.

Following this thread, I get the cue that acs-aem-commons widgets can be extended and tweaked. Please let me know the problem in my solution.

justinedelson commented 6 years ago

@mollymehta91 without more details, it is hard to say, but please note that "tweaked" and "extended" are different. If the only way to accomplish your need is to actually change the JS from Commons, I think copy/paste is your only option. The goal here was to allow the load order to be manually altered so as to ensure that a custom client library is loaded after Commons.