Support reparenting views during drag and drop without destroying and recreating them

[NewtMa]

Version: 1.46.0
Commit: a5d1cc2
Date: 2020-06-10T08:59:04.923Z
Electron: 7.3.1
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Linux x64 5.3.0-59-generic

Steps to Reproduce:

1.trigger the refactoring preview window;
2.drag it to wherever you like ---- oops, cannot drag them
and yes it can be reproduced on the Insider version

Does this issue occur when all extensions are disabled?: Yes/No
Yes

[sbatten]

@jrieken made it moveable, can you verify it functions as expected when moved

[sbatten]

To Verify:

1. Trigger Refactor Preview
2. Try to move it to a new location
3. Verify this works

[bpasero]

I find it weird:

• I can move the references view but when dropped into the sidebar, it does not get active
• the contents are lost (my refactoring preview)

[sbatten]

@jrieken can you please take a look at why the view is not preserving its state? I have made the view moveable and it seems to work but I don't think the view is handling rehydration properly.

[jrieken]

I don't know why it wouldn't keep its state. What's the API contract on a view? Is more than reparenting happening?

[jrieken]

Oh, wow. Debugged this. The view is disposed and re-created... That's unexpected because the refactor preview view has no way to "globally" infer its input. The view gets opened and set with input which it then expects to hold until refactoring is done...

[jrieken]

@sbatten What is the design here? Should all views keep their state externally so that they can get disposed and re-created? Is there other views from which I can do monkey-see-monkey-do?

[sbatten]

For most of our tree views, they get to keep state with the Tree Registry. Something that has state but doesn't benefit from that is Search. Search writes state so that the fields will remain upon re-creation, but it doesn't keep the search results state. @sandy081 and I talked about having some eventing for views to serialize and deserialize their state when moved, but it hasn't been a major issue as most views aren't that stateful it seems (at least not in any way different from their initial hydration steps). at present, views are on their own to preserve state.

[jrieken]

This is really unexpected and big challenge for the refactor preview because it works as "passive" view. What I mean is that it doesn't have a source from where to display data. There is a part that calls setInput on the view which then returns a promise with the approved or modified code action. This is very similar to a dialog or quick pick and the whole model is very transient, the view is only part of an operation and then hides itself. I don't see how dispose/re-create fits into this. How should this be communicated to the actual driver of the view? Must that listen to view dispose and re-create events?

[jrieken]

@sbatten I have reverted d41bbd3 and I have made this a feature request. Given the dispose/re-create approach this requires serious reworkings of the refactor view

[sandy081]

Discussed with @jrieken and it seems this view cannot be recreated using state as it is passive as Joh mentioned. This is similar to tree view that keeps its UI piece in the model and just re-attach it on view creation. I would suggest to pull this concept to views framework and let the consumers use this. In short this is reparenting views.