[MGNLUI-3868] Allow to provide a custom editor callback when creating a dialog Created: 29/Apr/16  Updated: 09/Feb/17  Resolved: 05/May/16

Status: Closed
Project: Magnolia UI
Component/s: framework
Affects Version/s: None
Fix Version/s: 5.4.7, 5.5

Type: Improvement Priority: Neutral
Reporter: Roman Kovařík Assignee: Roman Kovařík
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
dependency
is depended upon by PAGES-61 DefaultValue does not work for page p... 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:
Sprint: Kromeriz 42
Story Points: 3

 Description   

We can go to the next dialog when chaining dialogs and the related item doesn't have to be saved into JCR yet. This results into two problems:

  1. The content change event is triggered after closing each dialog.
  2. The tree is trying to select the item which doesn't exist yet. This results in a bug when adding a node at the root level since it tries to expand JCR root node (instead of the child which isn't saved yet) which is not part of the container.

Found after integration of PAGES-61.



 Comments   
Comment by Mikaël Geljić [ 03/May/16 ]
  • Exposing OpenCreateDialogAction's EditorCallback to subclasses makes sense to me
  • Those callbacks are a bit ugly anyway: they do the same thing onSuccess/Cancel on that very same object which initiates the callback (aka FormDialogPresenter). Perhaps it should just close itself eventually; while callbacks would be needed only for event firing. Not sure if we want to do that now, and what impact it would have though.
  • Finally, it seems to me that BrowserPresenter's handler should dismiss the event when a node doesn't exist, doesn't this happen? We'd probably want to fix this too then.
Comment by Roman Kovařík [ 03/May/16 ]

Finally, it seems to me that BrowserPresenter's handler should dismiss the event when a node doesn't exist, doesn't this happen? We'd probably want to fix this too then.

It tries to expand the parent node if the new node doesn't exist. It logs an error (in info.magnolia.ui.workbench.tree.HierarchicalJcrContainer#getParent) if you do it on the root level since the root node isn't part of the container. This is another bug than firing the event unnecessary twice.

Comment by Mikaël Geljić [ 03/May/16 ]

Yup, it's another thing, it depends how we see it because if we fix it, then firing the event twice would no longer be an issue I don't mind either way, do you want to create a separate ticket?

Comment by Roman Kovařík [ 04/May/16 ]

Separate ticket: MGNLUI-3874.
Firing the event twice is also an issue from my point of view, since the item is still in the creation process when firing the event for the first time. This can results in more problems if we add something else then expanding of items.

Comment by Mikaël Geljić [ 04/May/16 ]

Sure, thanks for filing

Generated at Mon Feb 12 09:10:54 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.