adobe / aem-spa-project-core

Contains abstraction for core Adobe Experience Manager and Single Page Application interface, the page.

Apache License 2.0

29 stars 29 forks source link

Add support for loading an experience fragment page in SPA editor mode. #26

Closed niekraaijmakers closed 3 years ago

niekraaijmakers commented 3 years ago

Description

This is done by adding a resource type that extends cq/experience-fragments/components/xf-page. Since this type does not support core components (it doesn't inherit the core component page), the model's core components functions don't work.

Therefore a JsonSerialze(as=...) is used to prevent errors being thrown in the model.json.

Motivation and Context

How Has This Been Tested?

I tested this in my implementation project with an experience fragment.

Screenshots (if appropriate):

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] I have signed the Adobe Open Source CLA.
[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

pfauchere commented 3 years ago

I am not certain that the current project should be used to compensate for the absence of Sling Models in "underlying" libraries, modules, packages, or bundles.

if HierarchyNodeExporter isn't required, the scope of the current project should remain the support of SPA SDK capabilities. We could instead enhance [0]

[0] https://github.com/adobe/aem-core-wcm-components/blob/master/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/models/ExperienceFragment.java

niekraaijmakers commented 3 years ago

Agree with above, however this path will take longer to walk. However it is the correct one

codecov[bot] commented 3 years ago

Codecov Report

Merging #26 (d6c048c) into master (9d7263e) will increase coverage by 6.90%. The diff coverage is 33.33%.

@@             Coverage Diff              @@
##             master      #26      +/-   ##
============================================
+ Coverage     70.98%   77.88%   +6.90%     
- Complexity       71       89      +18     
============================================
  Files             7        9       +2     
  Lines           193      199       +6     
  Branches         30       30              
============================================
+ Hits            137      155      +18     
+ Misses           39       27      -12     
  Partials         17       17

Impacted Files	Coverage Δ
...spa/project/core/internal/impl/RemotePageImpl.java	`0.00% <0.00%> (ø)`
.../adobe/aem/spa/project/core/models/RemotePage.java	`0.00% <0.00%> (ø)`
...e/aem/spa/project/core/internal/impl/PageImpl.java	`76.08% <66.66%> (+36.55%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ccd622e...d6c048c. Read the comment docs.

niekraaijmakers commented 3 years ago

I am not certain that the current project should be used to compensate for the absence of Sling Models in "underlying" libraries, modules, packages, or bundles.

if HierarchyNodeExporter isn't required, the scope of the current project should remain the support of SPA SDK capabilities. We could instead enhance [0]

[0] https://github.com/adobe/aem-core-wcm-components/blob/master/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/models/ExperienceFragment.java

HierarchyNodeExporter turns out not to be required. However having said that, the thing we need to modify here is the page (so we can edit it), not the component for inclusion.

The only (better) alternative is to have the experience fragment editor support mixins or something like that, so you don't have to inherit the resource type cq/experience-fragments/components/xfpage