Adding fallback for forms component #2568
Conversation
dmaurya929
commented
Aug 7, 2023
•
dmaurya929
commented
| Q | A |
|---|---|
| Fixed Issues? | FORMS-7969 |
| Patch: Bug Fix? | |
| Minor: New Feature? | |
| Major: Breaking Change? | |
| Tests Added + Pass? | Yes |
| Documentation Provided | Yes (code comments and or markdown) |
| Any Dependency Changes? | |
| License | Apache License, Version 2.0 |
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2568 +/- ##
=========================================
Coverage 87.34% 87.34%
Complexity 2648 2648
=========================================
Files 232 232
Lines 7080 7080
Branches 1073 1073
=========================================
Hits 6184 6184
Misses 359 359
Partials 537 537 ☔ View full report in Codecov by Sentry. |
|
@vladbailescu please help to review and merge this PR. |
gentle reminder |
|
Kudos, SonarCloud Quality Gate passed!
|
|
@dmaurya929 , is there a ticket for this PR anywhere, describing the issue, how to reproduce it etc.? |
| @@ -444,6 +444,10 @@ | |||
| subTitle = item[PN_PANEL_TITLE]; | |||
| } else if (item.title) { | |||
| subTitle = item.title; | |||
| } else if (item.label && item.label.value) { | |||
There was a problem hiding this comment.
Are item.label, item.label.value and item.name somehow specific to your custom component data model? I cannot find these properties in the component models provided in this project. Could you show me where do they come from?
There was a problem hiding this comment.
No, it's not specific to custom component. This is part of the headless specification of the forms core components. For instance look at [1].
[1]: https://aemcomponents.dev/content/core-components-examples/library/adaptive-form/emailinput.html#tabs-13ca5be0c7-item-46a8eee3d4-tab
There was a problem hiding this comment.
That's what I mean, it's specific to Forms Components, unrelated to Core WCM Components.
You can try fixing this in the component exporter of com.adobe.cq.forms.core.components.internal.models.v1.form.PanelImpl in a similar way to Core WCM Components, see [1], [2], [3].
[1] https://github.com/adobe/aem-core-wcm-components/blob/main/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/AbstractPanelContainerImpl.java#L119
[2] https://github.com/adobe/aem-core-wcm-components/blob/main/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PanelContainerItemImpl.java#L152
[3] https://github.com/adobe/aem-core-wcm-components/blob/main/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/models/v1/PanelContainerItemImpl.java#L196
There was a problem hiding this comment.
This will add an extra property cq:panelTitle in model json which will not adhere headless specification of forms. Other way would be to override this implementaion in Forms (it will require exposing class to namespace i.e CQ.CoreComponents.PanelSelector )
Does cq: prefix for wcm component mean some internal fields ?
cc: @rismehta
There was a problem hiding this comment.
@LSantha , in the context of forms, we follow a specific specification, and it's important that the JSON aligns with this specification. While we already align with certain WCM properties like :items and :itemsOrder, which are part of the public core component documentation, it's worth noting that cq:panelTitle is an internal WCM property. Therefore, I believe there might be a need for a client-side hook to override this panel title behavior since it's not part of any specification.
There was a problem hiding this comment.
Thanks @dmaurya929 , LGTM.
There was a problem hiding this comment.
LGTM. @vladbailescu @LSantha Could you review this, relevant PR in forms where we are using this an extension point, adobe/aem-core-forms-components#957 (review)
|
Kudos, SonarCloud Quality Gate passed!
|
