[MGNLFE-155] SSR | It should not be necessary to tell FEHelpers if it is in the page editor Created: 02/Dec/21  Updated: 17/Aug/23  Resolved: 28/Jun/23

Status: Closed
Project: Magnolia Frontend Helpers
Component/s: None
Affects Version/s: None
Fix Version/s: 1.4

Type: Improvement Priority: Major
Reporter: Christopher Zimmermann Assignee: Robert Šiška
Resolution: Fixed Votes: 0
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: 22.25d Time Spent: 22.25d
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Problem/Incident
is caused by MGNLFE-118 p13n support for SSR Closed
dependency
depends upon MGNLFE-232 Simplify usage of frontend-helpers wi... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLFE-470 Implement Sub-task Completed Robert Šiška  
MGNLFE-471 Review Sub-task Completed Phong Le Quoc  
MGNLFE-472 PiQA Sub-task Completed Phong Le Quoc  
MGNLFE-473 QA Sub-task Completed Oanh Thai Hoang  
MGNLFE-550 DOC: SSR | It should not be necessary... Sub-task Completed Martin Drápela  
MGNLFE-551 DOC: Note that global.mgnlInPageEdito... Sub-task Completed Martin Drápela  
Template:
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Documentation update required:
Yes
Date of First Response:
Epic Link: SaaS MVP - Project DevX
Sprint: DevX 38, DevX 39, DevX 40, DevX 41, DevX 42
Story Points: 0.5
Team: DeveloperX
Work Started:
Approved:
Yes

 Description   

Proposal

I thnk this can maybe be addressed once https://jira.magnolia-cms.com/browse/MGNLFE-232 is merged in.

Explanation

Currently it is necessary to tell the fehelper libraries whether the app is running in the page editor. Developers need to set global.mgnlInPageEditor to true in case rendering green bars for SSR.

The current approach was chosen in the interest of time and to ensure that  the developer had the control to set this information no matter what type of environment they are running in.

Now, with this ticket,  we invest the time to find a good solution that works with all key anticipated use cases. At least with SSR/Preview on Next, Gatsby, Scully and Nuxt.

See the ticket whhere behavirou was introduced: https://jira.magnolia-cms.com/browse/MGNLFE-118

Key comments:
"I think it's risky to remove the inIframe check now. On nodejs we have the global object that we can add something like mgnlEditMode=true"

"As long as it works in next js (or any other SSR) it's perfect.
We make it work, then we can make it better"



 Comments   
Comment by Christopher Zimmermann [ 20/Jun/23 ]

rsiska plequoc So can we wrap this one up then? Are we waiting on anything more?

Do you have any tips for the documentation team - what should be updated? mdrapela 

Comment by Robert Šiška [ 26/Jun/23 ]

mdrapela czimmermann We should remove all the mentions of global.mgnlInPageEditor in the frontend-helpers documentation.

(remove "or when global.mgnlInPageEditor is set to true." and the whole paragraph and example of "mgnlPreview")
The recommended approach should be the getMagnoliaContext section.

The second bullet point in Next.js-related aspects should be changed to:

2. Use EditorContextHelper.getMagnoliaContext:

...
  const magnoliaContext = EditorContextHelper.getMagnoliaContext(resolvedUrl, pageNode, languages);
  let props = {
      isMagnolia: magnoliaContext.isMagnolia,
      pagePath: magnoliaContext.nodePath
  }; 
  // fetch content
  const pagesRes = await fetch(pagesApi + magnoliaContext.nodePath + magnoliaContext.search);
  props.page = await pagesRes.json();

  // fetch annotations only when in page editor
  if (magnoliaContext.isMagnolia) {
    const templateAnnotationsRes = await fetch(templateAnnotationsApi + props.pagePath);
    props.templateAnnotations = await templateAnnotationsRes.json();
  }
...

Alternatively, you can fetch annotations only on client-side:

...
  const [templateAnnotations, setTemplateAnnotations] = useState(); 
  const { isMagnolia, pagePath } = props;
  useEffect(() => {
    async function fetchTemplateAnnotations() {
      const templateAnnotationsRes = await fetch(templateAnnotationsApi + pagePath);
      const templateAnnotationsJson = await templateAnnotationsRes.json();
     setTemplateAnnotations(templateAnnotationsJson);
    }
    if (isMagnolia) fetchTemplateAnnotations();
  }, [isMagnolia, pagePath]);
...

In the Next.js with Static Site Generation (SSG), I would propose to remove the whole third bullet point.

in Editor/preview check section:

..... Get the context object by calling

const magnoliaContext = EditorContextHelper.getMagnoliaContext(fullURL, pageRootPath, languages);

You can then use the following properties to get the status:

  • magnoliaContext.isMagnolia
  • magnoliaContext.isMagnoliaEdit
  • magnoliaContext.isMagnoliaPreview
Comment by Christopher Zimmermann [ 27/Jun/23 ]

Thanks rsiska !
Lets add a small note that `global.mgnlInPageEditor` is deprecated and not necessary. mdrapela 

Comment by Martin Drápela [ 27/Jun/23 ]

Note added:

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