[PAGES-352] Provide endpoint for template annotations for SPA rendering Created: 26/Aug/20  Updated: 07/Feb/24  Resolved: 11/Dec/20

Status: Closed
Project: Magnolia pages module
Component/s: None
Affects Version/s: None
Fix Version/s: 6.2.5

Type: New Feature Priority: Major
Reporter: Christopher Zimmermann Assignee: Robert Šiška
Resolution: Fixed Votes: 2
Labels: None
Remaining Estimate: 0d
Time Spent: 11d 1.75h
Original Estimate: Not Specified

Issue Links:
causality
caused by MGNLFE-67 React helper ignoring area title Closed
caused by MGNLFE-65 SPA Editor: Magnolia area dialog is d... Closed
caused by MGNLFE-66 SPA Editor: Areas in Magnolia show a ... Closed
caused by MGNLFE-68 I18n Labels for area / component titl... Closed
caused by MGNLFE-70 Frontend-helpers should support edita... Closed
caused by MGNLFE-64 Personalization variants not supporti... Closed
caused by MGNLFE-57 Allow more control on the HTML Commen... Closed
caused by MGNLFE-58 Allow using dialog at area level Closed
caused by MGNLFE-61 Support for user-defined area templates Closed
caused by MGNLFE-62 EditableArea is rendering a mgnlEdito... Closed
is causing PAGES-356 DOC: New template-annotations endpoint Closed
dependency
is depended upon by MGNLFE-77 Update React & Angular Editors to use... Closed
relation
is related to MGNLFE-74 Property maxComponents is not taken i... Closed
Template:
Acceptance criteria:
Empty
Documentation update required:
Yes
Date of First Response:
Epic Link: SPA Editor
Sprint: HL & LD 12, HL & LD 13, HL & LD 14, HL & LD 15, HL & LD 16, HL & LD 17
Story Points: 13

 Description   

Timebox for investigation:

  • How to pass the data/annotations? Does not necessarily have to REST

There are many new tickets being created that relate to the html annotations in the page that are then used by the page editor. A part of resolving those tickets will involve changing the frontend-helpers library to change the annotation code to add some aspect that is present in the original server-side annotation creattion code.

In the development of the Visual SPA Editor feature there was a choice to be made about how the annotations should be added with the frontend helpers. It was decided to re-implement the annotation creation in the frontend helpers, rather then to retrieve the annotations from the server.

Based on these new tickets, and the realization that new modules (live copy, abtesting, etc) can impact the annotations, we should reassess that decision and see if there is a better approach such that we won't have to continuously change the frontend helpers library to accomodate new features being added to Magnolia on an ongoing basis.

Please see linked "caused by" tickets to see issues that might be related to this topic.



 Comments   
Comment by Bartosz Staryga [ 28/Aug/20 ]

I am raising two hands for moving comment creation to server side.
The reason is not to get rid of template-annotation - this is only a nice side effect.
I have two resons:

  • Every prop used in green bars needs to be explicitly added to template-anotation lib. We already had to add the edit for dialog. Ecommerce headless demo is using patched version cause we had to add live copy props there. Personalization - not supported, noComponent - not supported, disabled - not supported (some more that we missed too, not supported?). Since we already have the nice piece of code that does all that comment buidling (used in freemarker), it is silly to try to rebuild it on JS side. Not only they are not kept in sync, we repeat the same functionality twice. Not to mention clients projects that add some extra stuff to green bars, they all would had to patch template annotation lib. So we close this route for any dev team who want to extend Magnolia without extra hustle.
  • We force extra lines of code on final budle size. This is not plenty, but if we can save few kb on js size we should do it.

Things like live copy or personalizations are bread and butter of every single sales demo we have right now, using patched code is not ideal

Comment by Christopher Zimmermann [ 09/Sep/20 ]

Re: "set the javascript object with all annotations into the page". At some points I found it helpful to be able to do some debugging of the green-bar creation directly on the React dev-server, which is only possible with it as a REST API. I guess a change like that would also "break the contract" and devs would need to update their projects, so it would necessitate a new major version, which would be nice to avoid.

Comment by Bartosz Staryga [ 09/Sep/20 ]

+1 to what czimmermann says. Leave REST, it's not overhead, and it still leaves the ability for geeky projects to do some previews with e.g. some extra marking of components etc.
simply making rest return: e.g.
[ ...,
{
PATH_TO_COMP:

{ open: OPEN_COMMENT_STRING, close: CLOSE_COMMENT_STRING }

},
...]
will do the trick.

Comment by Canh Nguyen [ 05/Oct/20 ]

mdrapela for updating the document:

  • The new endpoint is /.rest/template-annotations/v1/
  • We need to add mgnlPreview=false to the URL to make it work because the RenderEngine will check this param to render freemarker templates. mgnlPreview is true by default if we omit it.
  • Components need hardcode template scripts to be rendered, so we have to add following line to component definitions

 

class: info.magnolia.rendering.spa.renderer.SpaTemplateAnnotationRenderableDefinition

 

Comment by Christopher Zimmermann [ 05/Oct/20 ]

canh.nguyen is there any way we can implement this so that developer does not have to add 

class: info.magnolia.rendering.spa.renderer.SpaTemplateAnnotationRenderableDefinition

 to every component definition? If so - would there be additional "cost" for us in additional effort now and maintenance?

For one, we want the definitions as simple as possible. Additionally, this will be kind of intimidating for some frontend developers.

Comment by Bartosz Staryga [ 05/Oct/20 ]

canh.nguyen +1 to what czimmermann says. Big +1.

Comment by Canh Nguyen [ 06/Oct/20 ]

czimmermann I guess we can omit the class by implementing it like PersonalizationTemplateDefinition. But there is a problem that if I extend PersonalizationTemplateDefinition class, pages module will require personalization module (I've not check dependency loop yet). If we don't extend PersonalizationTemplateDefinition class, we will lose that functionality.

mgeljic and apchelintcev could you please give me some advice for this issue?

Comment by Mikaël Geljić [ 06/Oct/20 ]

Re: personalization dep in pages, that would be a no-go but there would be various ways to invert that dependency.
As discussed, let's review the general approach here.

Comment by Robert Šiška [ 24/Nov/20 ]

Update for docu:

  • The new endpoint is /.rest/template-annotations/v1/
  • **No template definitions need to be changed.
  • Page template def. still needs to declare renderType: spa and class: info.magnolia.rendering.spa.renderer.SpaRenderableDefinition
Comment by Bartosz Staryga [ 24/Nov/20 ]

Quick question, do I see correctly that I would have to change the `class` and the endpoint to use newer approach?
How would it look on fe side?

Is there a possibility that this whole change is more graceful and handled behind the scenes, so the same class endpoint etc, all stays the same, and handling is handled in fe editor libs? Eg. endpoint could return some flag and fe libs would process it accordingly, and we could give a grace period for users to migrate to latest and then remove old code.

I just hate to ask again our partners/clients that if they wanna use something newer they need to modify their code again... (i know they will have to update fe libs, but we would at least save them from need to update yaml. small gesture but nice )

czimmermann what do you think?

Or we communicate it as /templateDefinition/v2, after all this is why we have versioning i guess

Comment by Robert Šiška [ 24/Nov/20 ]

The class stays the same as it was before. And it needs to be there because it defines jsPaths and cssPaths attributes.

Or we communicate it as* /templateDefinition/v2*, after all this is why we have versioning i guess

That's a good point, but I wanted to make it obvious that the endpoint has different purpose - it returns annotations rather than template definitions. 

And it only needs to be changed on one place, so I reckoned it's not such a big deal. wdyt?

The old definition endpoint still works, by the way, so nothing should break after update.

Comment by Christopher Zimmermann [ 24/Nov/20 ]

I just checked one of my code examples - there in the page definition i have:

class: info.magnolia.rendering.spa.renderer.SpaRenderableDefinition

Can I still use that rsiska? Because you mention a different one in above comment.

Comment by Bartosz Staryga [ 24/Nov/20 ]

hmmm the class I have in current ones is:
class: info.magnolia.rendering.spa.renderer.SpaRenderableDefinition
you say new one is:
class: info.magnolia.rendering.spa.renderer.ConfiguredSpaRenderableDefinition
is that some new class from previous release I missed? LOL

Then only question I have is, why not: /templateDefinition/v2? Why completely new endpoint?

Comment by Christopher Zimmermann [ 24/Nov/20 ]

I think the endpoint renaming is reasonable since it really does return different information (not just definitions anymore - YAY). (And same amount of work for the FE developer either way.)

Comment by Robert Šiška [ 24/Nov/20 ]

Ah, you're right. You can (and should) use info.magnolia.rendering.spa.renderer.SpaRenderableDefinition.

Why completely new endpoint?
I edited my previous comment. TLDR: because it doesn't return template definitions anymore.

Comment by Bartosz Staryga [ 25/Nov/20 ]

rsiska
Regarding the endpoint name
Fair enough
Just to shed some light why I'm writing what I am writing is cause for every developer I came across that had to implement that bit in FE this is some arbitrary code that needs to be added (in 90% cases copy-pasted from our examples) so we have green bars.
There is either very small understanding what is being fetched there or majority has 0 understanding of it at all.
Anyway make sense what you are saying

So question more visual then:
Old one is called: templateDefinition and new one template-annotations. What is the preferred way of naming endpoints in PD? Camel case or dashes?

Would you mind sharing here example response from that endpoint here please?

Comment by Canh Nguyen [ 26/Nov/20 ]

bstaryga we've changed to template-definitions long ago, but we continue support templateDefinition for backward compatibility.

Comment by Bartosz Staryga [ 26/Nov/20 ]

canh.nguyen hehehe, we missed it all at FE team then
I shall update all our materials accordingly then.

Comment by Christopher Zimmermann [ 30/Nov/20 ]

Fix version is 6.2.5, Magnolia version is 6.2.6. What version is this in? Was it shipped with 6.2.5?

Comment by Robert Šiška [ 01/Dec/20 ]

It was reverted in the final QA and because it is the only change in pages 6.2.5 so far, the module wasn't released. Magnolia 6.2.5 was released with pages 6.2.4.

Generated at Mon Feb 12 06:18:06 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.