[PAGES-514] Drop "class" prop for "renderType: spa" Created: 30/Sep/21  Updated: 14/Mar/22  Resolved: 14/Mar/22

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

Type: Improvement Priority: Neutral
Reporter: Bartosz Staryga Assignee: Unassigned
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PAGES-560 SPA renderer supports site prototype,... Closed
relates to PAGES-548 Create $type annotation for SpaRender... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   

Currently for renderType: spa Page template def must have class prop.

title: SPA Page
dialog: spa-light-module:pages/default
templateScript: /spa-light-module/webresources/index.html
renderType: spa
class: info.magnolia.rendering.spa.renderer.SpaRenderableDefinition
areas:
  (...)

Could we drop the need for class prop, since we already have spa as renderType?

To have something like:

title: SPA Page
dialog: spa-light-module:pages/default
templateScript: /spa-light-module/webresources/index.html
renderType: spa
areas:
  (...)

It looks less scary and more dev friendly



 Comments   
Comment by Christopher Zimmermann [ 30/Sep/21 ]

jsimak would this be possible? Would there be some negative reprecussions?

Comment by Christopher Zimmermann [ 30/Sep/21 ]

An alternative could be to create an "alias" for the class so that instead of using the class, dev would include something like:

$type: SpaRenderable

Customers would probably still find it a bit redundant, but it would be an improvement right bstaryga?

Comment by Jaroslav Simak [ 30/Sep/21 ]

Dropping the class is not possible. Having a $type could be a way to go.

Comment by Bartosz Staryga [ 30/Sep/21 ]

Why in freemarker it's not needed and here it is not possible?
Having renderType and $type/class is potato, potahto. Not really an improvement

Comment by Adrien Manzoni [ 01/Oct/21 ]

Hi jsimak

Would it be possible to add the 3 properties defined in info.magnolia.rendering.spa.renderer.SpaRenderableDefinition into info.magnolia.rendering.template.configured.ConfiguredTemplateDefinition ?

Then you would not need to define that custom class, wouldn't you ? 

Comment by Christopher Zimmermann [ 14/Feb/22 ]

I've further reviewed this with PD and we will not do this in the near term. I leave the ticket open as we should consider this again in a new Major version of Magnolia (6.3? 6.4?) We can consider what the defaults should be and how definitions should change in a bigger context.

For now we add a type alias for the SPA class to make it easier and cleaner to read:

https://jira.magnolia-cms.com/browse/PAGES-548

Comment by Christopher Zimmermann [ 14/Mar/22 ]

Further investigated internally and deemed this as something that would be confusing to maintain and could cause issues in further feature efforts in the future.
Therefore won't do - but we do make this related improvement: https://jira.magnolia-cms.com/browse/PAGES-548

Comment by Christopher Zimmermann [ 14/Mar/22 ]

But again - thanks for the suggestion to improve developer experience.

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