adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
733 stars 742 forks source link

Dialog link V1 #2265

Open fahol-coop opened 2 years ago

fahol-coop commented 2 years ago

Bug Report

Current Behavior Every Component that is using the Link Dialog ("/apps/core/wcm/components/commons/editor/dialog/link/v1/link/edit/.content.xml") like Button V2 or Image V3 has the Problem that you can't choose Files from Assets in the link pathfield. It was changed from Pathfield ("granite/ui/components/coral/foundation/form/pathfield") to Pagefield ("cq/gui/components/coral/common/form/pagefield"), and the exceptions for the files in property "nodetypes" are not working with the "Pagefield"

Expected behavior/code In URL field, selection of Assets should be possible. As there are already definition in Property "nodetypes" to allow Assets.

Environment

Possible Solution Just go back to "Pathfield", that the Property for "nodetypes" will work. Or make the notetypes Property work with "Pagefield"

Additional context / Screenshots Add any other context about the problem here. If applicable, add screenshots to help explain.

image

https://github.com/adobe/aem-core-wcm-components/blob/main/content/src/content/jcr_root/apps/core/wcm/components/commons/editor/dialog/link/v1/link/edit/.content.xml

bpauli commented 2 years ago

@fahol-coop to links Assets we have the dedicated Download Component which comes with the fileupload widget which comes with more features then the pathfield widget.

Can you please provide feedback in case the download component doesn't meet you expectation? Thanks!

fahol-coop commented 2 years ago

Yes I saw this component, but is's a separate one. https://www.aemcomponents.dev/content/core-components-examples/library/core-content/download.html

The Component itself for downloading files and Highlighting it with title and size is a good thing. But if you need to Combine it, for example with a call to action button in teaser, it will not work until you create a customize call to action button in teaser. There are also a lot of informations, they are nice, but if you just want solution for adding a pdf to an image, or an button component, then those informations are not needed or maybe will not fit to the context.

I think there are two different situations.

The Pathfield or the possibility to add a asset File to an Image, Button, Teaser and so on is realy important for simple things that are already existed before, where you don't need all that fancy stuff, you just need the customer to download a pdf (like menu).

And the Download Component itself is nice for special situations. Or do you plan to combine the download component in Teaser, Button , Image, List and so on? And how will this look like? A teaser with a call to action download button with a title on top? I think it should not get mixed together.

fahol-coop commented 2 years ago

Are there any news?

bpauli commented 1 year ago

@adobe export issue to Jira project SITES as Story

ackoch commented 1 year ago

@fahol-coop : I don't think, this is a simple change.

According to https://github.com/adobe/aem-core-wcm-components/issues/904 and https://github.com/adobe/aem-core-wcm-components/issues/1333 that was a deliberate decision. Just changing it back won't help .. otherwise we would constantly PR contradicting changes.

The simple change reverting back to the original behaviour works... but this might result in a more "messy" picker when suddenly users see resources below /content that have not been there before (content often is abused as a kitchen sink for arbitrary configurations... )

Imho, that should be a broader discussion.

You mentioned, that the property nodyTypes:"dam:Asset" is ignored. That seems to be the root (and solution of the issue). The pagefield (and the according picker) should be fixed/augmented to take that parameter into account (not revert to pathfield). As the page field is built-in Adobe component, you could try opening a support ticket ;-)

fahol-coop commented 1 year ago

This looks like a big mistake that happened here. Some People tried to warn the core Team not to change the Pathfield to Pagefield without checking the different use cases. https://github.com/adobe/aem-core-wcm-components/issues/904

The Warning was ignored, and now we are in this unsatisfied Situation.

At least there had to be a Dialog V2 for this breaking change.

pgoodrich commented 1 year ago

@bpauli @ackoch There are many sites that want to link to and display assets inline instead of downloading. The current implementation is very opinionated.

I thought @ky940819 had a good suggestion here: ky940819 https://github.com/adobe/aem-core-wcm-components/issues/1333#issuecomment-802466267

Perhaps only pages and assets as a compromise for now? This would also resolve #1333

fahol-coop commented 8 months ago

Are there any news?