[MGNLUI-4026] Site app's move dialog doesn't work correctly Created: 26/Sep/16  Updated: 09/Feb/17  Resolved: 19/Oct/16

Status: Closed
Project: Magnolia UI
Component/s: dialogs
Affects Version/s: 5.5
Fix Version/s: 5.5

Type: Bug Priority: Major
Reporter: Hieu Nguyen Duc Assignee: Sang Ngo Huu
Resolution: Fixed Votes: 0
Labels: usability, ux, working-with-items
Remaining Estimate: 0d
Time Spent: 2.25d
Original Estimate: 5d

Attachments: PNG File screenshot.png    
Issue Links:
causality
caused by MGNLUI-3898 Reveal moved item in the "move" dialog 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)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Saigon 66
Story Points: 8

 Description   

Steps to reproduce:
+ Go to Site app
+ Select an item
+ Choose "Move item"

=> Expected: It shows tree structure correctly.
=> Actual: It shows "extends" property (see "screenshot.png")



 Comments   
Comment by Mikaël Geljić [ 10/Oct/16 ]

For the records, MGNLUI-3898 substituted the configured TreePresenterDefinition implementationClass—with a move-specific presenter impl—effectively ignoring apps with such customized presenter implementationClass (like the Sites app).
The proposed legitimate approach now goes along "wrapping over substitution", so far so good.

In details, WorkbenchPresenter, among other components, does not expose any hook for wrapping instances of configured content-presenter impls.
Current PR goes around this limitation by mutating the original definition to substitute it with a wrapper implementation, we then lose access to the originally configured implementationClass too. So it's kind of wrapping applied in non-wrapping ways, for the lack of any better & concrete alternative. We debated options with apchelintcev:

  1. As proposed on the PR: hack #getOriginalImplementationClass via subAppContext and down again
    • stands, given the move dialog is intended to show the same context as the browser beneath it
  2. mutate original definition to proxy/give access to the original implementationClass through another field
  3. implement and expose mutator unwrapping feature
  4. expose hook in WorkbenchPresenter to perform wrapping "in the right place"
    • doesn't solve how to properly wrap WorkbenchPresenter in the first place
  5. flatten reveal into default impl, don't wrap
  6. more power to the views?
    • i.e. presenter configuring the view just once; view becoming more autonomous
    • can be regarded as displacing the problem as long as wrapping can't be plugged properly

Though we picked the hack over fancier solutions, we're effectively buying some time to rethink similar—widely used—patterns in broader ways (this has been mentioned over and over again).

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