Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dragging OutputAreaWidget produces new OutputAreaWidget mirroring same model #424

Merged
merged 6 commits into from Jul 19, 2016

Conversation

danielballan
Copy link
Contributor

@danielballan danielballan commented Jul 16, 2016

Drag-and-drop any notebook output areas to generate a new stand-alone widget that mirrors that output area.

Thanks for the hand, @sccolbert.

output_dragdrop

@danielballan
Copy link
Contributor Author

If the origin cell is deleted, one is left with an empty mirror widget. What is the proper way to detect that the model is dead and dispose of the extra widget?

@jasongrout
Copy link
Contributor

@blink1073 and I were talking about this. A model could, for example, have a signal that it is being disposed. A widget could listen to that signal and dispose itself when the model is being disposed. Then it would be on the owner of a widget, if it is changing the widget's model, to change the model before disposing the old model. @sccolbert, what do you think?

@danielballan
Copy link
Contributor Author

As written, this breaks slider widgets. The slider sees the mousemove event, but it propagates it to the OutputWidgetArea, and a Drag is started. Should jupyter-js-widgets block propagation of events to parents?

@sccolbert
Copy link
Contributor

We should probably be more selective on which parts of the output area are valid for starting a drag. Since an output area can contain arbitrary output, we should probably not consider rendered output as a viable drag-start target. Maybe restrict the target area to the left side gutter?

@danielballan
Copy link
Contributor Author

@sccolbert That makes sense to me. See new commit, defining a widget class, OutputGutter, and moving the _startDrag logic there.

@sccolbert
Copy link
Contributor

@danielballan can you post a gif of your most recent commit? If you need a tool to record it, most all of us use this one: http://www.cockos.com/licecap/

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Jul 17, 2016

This is awesome. Adding a semantic class name to the mirrored output would help styling it differently (for example not display the top-level prompt in the mirrored output).

Also changing the mouse pointer when hovering the output gutter would help make this discoverable.

@danielballan
Copy link
Contributor Author

Sure. This recording demonstrates two things that work and two things that don't.

Good:

  1. Dragging from the left-side "gutter" of an output area generates a "mirrored" copy that watches the same model.
  2. Dragging from anywhere else in the main output area now does nothing, which avoids interfering with IPython widgets or other interactive elements there.

Bad:
3. Deleting a cell leaves any extant mirror widget(s) blank.
4. IPython widgets do not appear in mirror widgets.

demo-424-1

Addressing (3), I wrote (unpushed) code in line with @jasongrout's suggestion above. When an OutputAreaModel is disposed, a new Signal will cause the corresponding OutputAreaWidget to be disposed. But I'm not sure what signal to watch to get this process started. Contrary to my expectation, deleting a cell does not seem to dispose that cell's OutputAreaModel.

I don't know where to start with (4). Is the problem obvious to anyone?

@danielballan
Copy link
Contributor Author

I think I solved (3) cleanly. See latest commit and this screencap of it working. I still could use help with (4).

demo-424-2

@blink1073
Copy link
Member

@danielballan, you need to explicitly set the "trusted" attribute of the mirrored widget, the "widget" mimedata is being stripped.

@danielballan
Copy link
Contributor Author

Thanks, @blink1073. That did it.

So, this is now working and ready for review / discussion about whether it is a good idea. :- )

demo-424-3

@sccolbert
Copy link
Contributor

The other point of discussion (and maybe we want to handle this in a separate PR) is the UI/UX to indicate the output drag target area. As it currently sits, the draggability (is that a word) of the output area is somewhat undiscoverable. We may want to indicate it visually somehow, along with perhaps making a command to "mirror" the output of the selected cell in a new pane (maybe also another PR).

cc @ellisonbg for UI/UX input

@sccolbert
Copy link
Contributor

Also, since it may not be apparent from my typing: THIS.IS.AWESOME!

@danielballan
Copy link
Contributor Author

Haha, thanks @sccolbert. All of the above sound reasonable to me -- including "draggability" as a word.

* A signal emitted when the model is disposed.
*/
get disposed(): ISignal<OutputAreaModel, void> {
console.log('get model disposed signal')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove debug print.

@blink1073
Copy link
Member

Great first PR, @danielballan, you nailed all of our design patterns.

@blink1073
Copy link
Member

Awesome, I added a demo GIF to your top level comment, will merge once Travis agrees.

@blink1073 blink1073 merged commit aebb750 into jupyterlab:master Jul 19, 2016
@danielballan danielballan deleted the dragable-output-area branch August 7, 2016 20:39
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants