[MAGNOLIA-4488] When navigations are moved in an area their links are no more clickable in edit mode, we must switch to the preview mode if we want to navigate. Created: 24/Jul/12  Updated: 26/Sep/12  Resolved: 26/Sep/12

Status: Closed
Project: Magnolia
Component/s: page editor
Affects Version/s: 4.5
Fix Version/s: 4.5.5

Type: Improvement Priority: Neutral
Reporter: Samuel Schmitt Assignee: Ondrej Chytil
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File pageeditor.patch    
Issue Links:
Cloners
is cloned by MGNLTPLSMPL-2 CLONE -When navigations are moved in ... Closed
is cloned by MGNLUI-9 CLONE -When navigations are moved in ... Closed
causality
relation
is related to MGNLUI-511 page editor: links inside page editor... Closed
Template:
Patch included:
Yes
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   

It's a pain to always switch to the preview mode when we want to click on a navigation item.

It be great if all the links wrapped in an element with an attribute role="navigation" stay clickable in the edit mode.



 Comments   
Comment by Boris Kraft [ 24/Jul/12 ]

I think that keeping the site navigation items clickable if nothing major speaks against it (like it not being editable) is ok, and also ok in a minor release to address it, as all we do is ensure that the site navigation works everywhere.

However I am against adding any additional elements to make that work, as the click-ability of nab items very likely will change in the future anyways. In other words, if what we do now is ensure all site navigation items work, not just some, that is fine.

Comment by Espen Jervidalo [ 24/Jul/12 ]

Attached a patch which stops processing Dom-Element and its children when an attribute role="navigation" is found.

In case you still need to be able to edit areas/components inside this elements, the patch needs to be adapted further.

Comment by Jan Haderka [ 24/Jul/12 ]

Fine then, but definitively not for 4.5.4. It's too close to the release date and there won't be enough time to test it ... and to write the fix including the test.

Comment by Ondrej Chytil [ 18/Sep/12 ]

Tickets for port commit - MGNLUI-9 and MGNLTPLSMPL-2.

Comment by Jan Haderka [ 26/Sep/12 ]
       boolean proceed = true;
       if(process) {

Since you use the proceed only inside of the {{if(){} }} block you should define the variable only inside. No need to create object and allocate memory when you will not use it.

Also since all the execution code is wrapped in the if block, it might be more readable to turn the code around and have at the top check

if (!process) {
  return;
}

to make it clear that if the flag is not set, method is not executed and not to have all other code deep in the loops.

Unrelated to the fix, but still related to the code:
Why is process defined as public?

 // In case we're in preview mode, we will stop processing the document, after the pagebar has been injected.
 111     public static boolean process = true;
Generated at Mon Feb 12 03:56:16 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.