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

Add External output area #825

Merged
merged 7 commits into from
May 31, 2017
Merged

Conversation

lgeiger
Copy link
Member

@lgeiger lgeiger commented May 29, 2017

This is a initial implementation of a external output display using docks:
external

If the output pane is present, output is redirected to this pane, only the spinner and status icons stay in the text editor.

I don't know what the best behavior for this panel would be. We can iterate one this an collect feedback from users.

Fixes #760

@rgbkrk
Copy link
Member

rgbkrk commented May 29, 2017

"Love the sidecar. Keeps things tidy." - @ivanov

@ivanov
Copy link
Member

ivanov commented May 30, 2017

As Kyle said, this looks great, @lgeiger! Another reasonable possibility for the outputs would be too keep adding them onto a single page on the left, and scroll down so that the last one comes into view... that way you can scroll down through them and see multiple (for example, if you're just printing one line per output, it'd be nice to see a bunch of them as opposed to having to click to page through them.

On the other hand, the current implementation is perfect for comparing images, flipping back and forth between them, so there are use cases for both.

@lgeiger
Copy link
Member Author

lgeiger commented May 31, 2017

@ivanov Thanks a lot for the feedback.

Yeah I though about this too. I didn't go with it because a infinite scrolling list might become a bit messy with time, especially with multiple rich outputs.

One thing to note is that we accumulate stdout (primarily for properly handling ANSI escapes):

stdout

I think this behavior is a good compromise between the ability of comparing multiple rich outputs while allowing users to read large amounts of stdout without constantly flipping through the history.


import typeof store from "../store";

const OututArea = observer(({ store: { kernel } }: { store: store }) => {
Copy link
Member

Choose a reason for hiding this comment

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

const OutputArea

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Good catch

);
});

export default OututArea;
Copy link
Member

Choose a reason for hiding this comment

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

OutputArea

Copy link
Member

@BenRussert BenRussert left a comment

Choose a reason for hiding this comment

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

I've had good results testing this out. Great work!

One thing I've been thinking about is how to handle having both watches and output panes open, ie could one or the other be pinned or should they split and use the same dock etc. But I agree there will probably be iterations from here.

@BenRussert BenRussert merged commit c37e72f into nteract:master May 31, 2017
@lgeiger lgeiger deleted the external-output branch May 31, 2017 14:55
@lgeiger
Copy link
Member Author

lgeiger commented May 31, 2017

@BenRussert The great thing about our new panels is that users can re-arange them however they like:
ezgif-2-19fc5f53bc

Totally agree, this is only the start. I'm happy to explore different UI's (e.g. a long scrolling list, or different output areas for different mime types, etc.)

@BenRussert
Copy link
Member

@lgeiger we should save that gif for the release notes and docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Output in separate pane
5 participants