livebook icon indicating copy to clipboard operation
livebook copied to clipboard

Add output panel

Open jannikbecher opened this issue 1 year ago • 27 comments

(created a new PR since upstream had some changes and new implementation differs a lot from the initial one)

jannikbecher avatar Jul 25 '23 13:07 jannikbecher

Uffizzi Ephemeral Environment deployment-33510

:cloud: https://app.uffizzi.com/github.com/livebook-dev/livebook/pull/2109

:page_facing_up: View Application Logs etc.

What is Uffizzi? Learn more!

github-actions[bot] avatar Jul 25 '23 13:07 github-actions[bot]

Some open questions:

  • order of outputs can only be changed by updating the order in the notebook, correct?
  • every output has its own row in the output panel, so the buttons to move output to the panel are obsolete?
  • outputs can only be dragged on the direct previous output?
  • basically I can use the drag&drop behaviour similar like @jonatanklosko used for file inputs without an external library?
  • where should I store the width information for the outputs?
  • does it make sense to use tailwinds column grid layout and use different breakpoints for different screen sizes?
<div class="grid grid-cols-1 md:grid-cols-6">
  <div :for={output <- outputs} class={"col-span-#{output.width}"}
    <%= output.content %>
  </div>
</div>

jannikbecher avatar Jul 25 '23 13:07 jannikbecher

order of outputs can only be changed by updating the order in the notebook, correct? every output has its own row in the output panel, so the buttons to move output to the panel are obsolete? outputs can only be dragged on the direct previous output?

Sorry if that wasn't clear, we still need the move button, the outputs can be cherry-picked and rearranged. It's just that rather than an arbitrary grid, we want it to be a flow of rows, with an option to put several outputs in a single row.

basically I can use the drag&drop behaviour similar like @jonatanklosko used for file inputs without an external library?

I think we should be fine without a library, yeah, unless it turns out there's some specific complexity that a library could handle.

where should I store the width information for the outputs?

I would add a field to %Notebook{} like outputs_panel, and it could be a configuration like %{rows: [%{items: [%{width: 50, cell_id: "..."}]}]} (where width is a percentage, which we can quantize to be every 10 percent or similar). We will persist this differently (because cell_ids are random on every import), but we can handle this once everything else is figured out.

does it make sense to use tailwinds column grid layout and use different breakpoints for different screen sizes?

Whatever is more convenient. I think on small screens we will just make every row into a column, so each output takes the whole width.

jonatanklosko avatar Jul 25 '23 16:07 jonatanklosko

Implemented Drag&Drop for reordering the rows:

screen-capture (10).webm

jannikbecher avatar Jul 26 '23 13:07 jannikbecher

@jannikbecher sweet! For the DND reordering we probably don't want to move the whole element, because I'm pretty sure it's not going to work well with the absolutely positioned iframes. Instead, we can drag a placeholder element, similar to this.

jonatanklosko avatar Jul 26 '23 13:07 jonatanklosko

Something like this?

screen-capture (11).webm

jannikbecher avatar Jul 26 '23 15:07 jannikbecher

Added multi column support :) Currently the space is split up equally among all items. When dragging on the left side of an item, it goes left. When dragging on the right side, right. Still many UI challenges, but is it the direction you imagined?

https://github.com/livebook-dev/livebook/assets/11441198/3de3c2f0-c46b-475f-81ab-fd0fce020ad3

jannikbecher avatar Jul 27 '23 13:07 jannikbecher

Looks like the right direction to me, yeah!

cc @josevalim @hugobarauna

jonatanklosko avatar Jul 27 '23 17:07 jonatanklosko

Looks like the right direction to me as well. 👍

Amazing! ❤️

hugobarauna avatar Jul 27 '23 20:07 hugobarauna

Looks beautiful! 😍

josevalim avatar Jul 28 '23 07:07 josevalim

Added deployments and iFrames :) Sorry for the bad video quality. I still try to find a good screen recorder for linux.. :-D Still many UI challenges, but I think the direction is clear :)

Untitled_ Jul 28, 2023 11_29 AM.webm

jannikbecher avatar Jul 28 '23 09:07 jannikbecher

Sorry for the bad video quality. I still try to find a good screen recorder for linux.. :-D

No worries, btw Peek worked quite well for me in the past :)

jonatanklosko avatar Jul 28 '23 11:07 jonatanklosko

Most things should be implemented for now. Tomorrow I will do some testing and try to fix the iframe hover and maybe some UX improvements. Unfortunately I won't be able to work on this for for the next two weeks. When I'm back I will definitely continue on this! Thank you so much for all the clear explanations/instructions and also for this beautiful language and code base! I had a blast working on this and can't wait to continue :smile: :heart:

jannikbecher avatar Jul 28 '23 14:07 jannikbecher

Awesome @jannikbecher! Would you be ok if one of us take it over the finish line in the next 2 weeks? I am not saying we will and we would also be glad to wait (so feel free to say no), but I thought I would ask in case we want to include it in the next release. :)

josevalim avatar Jul 29 '23 08:07 josevalim

Awesome @jannikbecher! Would you be ok if one of us take it over the finish line in the next 2 weeks? I am not saying we will and we would also be glad to wait (so feel free to say no), but I thought I would ask in case we want to include it in the next release. :)

Hey @josevalim, thanks for asking (not that I think you'd have to :-D) and I'm fine with either direction :)

jannikbecher avatar Jul 29 '23 10:07 jannikbecher

One nasty bug: It is not possible to check on dragenter if the drag element comes from outside or from another element. So when moving from the output panel to the notebook, the files dragenter event gets fired. So when the output panel is embedded and drag an item to the left side, livebook thinks you dragged a new file to the notebook.

But all in all I'm quite happy with the current implementation. There is definitely much room for UI improvements, but I think it is good enough to play around with it :)

jannikbecher avatar Jul 29 '23 16:07 jannikbecher

https://github.com/livebook-dev/livebook/assets/11441198/44c1d574-7192-4d4c-bad7-2404561d29bd

jannikbecher avatar Jul 29 '23 16:07 jannikbecher

Would you be ok if one of us take it over the finish line in the next 2 weeks?

We pushed Livebook v0.11 to after ElixirConf US, so we have a month until next release, so no rush from our side :)

josevalim avatar Aug 07 '23 18:08 josevalim

Hi @jannikbecher! Just a heads up our release probably be on the week of Sep 18th. However we will likely be tidying up features in the week of Sep 11th. :)

josevalim avatar Aug 31 '23 10:08 josevalim

hey @josevalim. Thanks for the update. Could you provide feedback on what needs to be improved for this to be merged into the main branch? :)

jannikbecher avatar Aug 31 '23 11:08 jannikbecher

@jannikbecher this looks amazing. I have bunch of nitpicks for us to iron out the final details (fwiw I have tested these on Firefox).

  1. Instead of showing the the trash icon along-side the drag and drop icon, show the logout box icon, since we don't really delete it, simply we put it back

  2. When I mouse over an element in the panel, scrollbars appear (see image below)

    Screenshot 2023-08-31 at 20 57 15
  3. When I start to drag and drop, a blue bar is used to represent the element, but I also have a blue bar in the same place as the drag and drop button (see image below)

    Screenshot 2023-08-31 at 20 58 50
  4. Can we unify the row and column drag and drops? Today I need to move something to a row and then move the columns. It would be nice if we could move everything around with a single drag and drop, something like this:

    |---------------------|
    |                     |
    |        row 1        |
    |                     |
    |---------------------|
    
    |----------|----------|
    |  row 2A  |  row 2B  |
    |----------|----------|
    

    Now imagine I move drag and drop row 2A. If I move a little bit up, we will see an area called new row which we could drop into:

    |---------------------|
    |                     |
    |        row 1        |
    |                     |
    |---------------------|
    |------ new row ------|
    |----------|----------|
    |  row 2A  |  row 2B  |
    |----------|----------|
    

    We need some "natural space" between rows but, once we mouse over it during drag and drop, it gets some height (and some color).

    If I move it a bit further up, we will see this:

    |---------------------|
    | |-------| |-------| |
    | |      r|w|1      | |
    | |-------| |-------| |
    |---------------------|
    
    |----------|----------|
    |  row 2A  |  row 2B  |
    |----------|----------|
    

    In other words, it will overlay two squares on top of the current content on row 1. If the drag and drop is on the left, we will show "New column" on the left, if on the right, we will show "New column" there.

I have navigated https://datasciencenotebook.org/ looking for inspirations and I think Hex' App Builder is the closest to what you are building, in case you are looking for inspiration in this final stretch. :)

josevalim avatar Aug 31 '23 19:08 josevalim

Continuing:

  1. Keep in mind that there is a button called "Clear outputs" (on the triple dots close to the notebook title), which means the outputs can be cleared at any time (this will also happen if you import a notebook). In such cases, we should probably show a box saying "This cell was not yet rendered". if you want to be fancy, you could even have a evaluate button alongside the drag and drop/delete buttons (we could leave this button permanently there. If the cell was never evaluted, we show the "play" icon. if it was evaluated, we can show the "memories" remix icon)

  2. Because of the above, the export button (login/logout) should always be visible, even if the cell was not evaluated

  3. There is additional extra space in the deploy modal if "Output panel" is chosen:

    Screenshot 2023-08-31 at 21 46 29
  4. Btw, I think we should change "Output type" to radio. It means users can see all options without an additional click. Especially given we only have three options


@jonatanklosko, quick question for you: what do we do with the placement of the export button (login/logout icons)? Currently it is on the right, but we have avoided adding icons to the right so the delete icon is always aligned. At the same time, I think on the right is the best place. What to do? Maybe we should allow Markdown to be sent to the output pane too? 🤐 It would make things consistent but maybe way more complex.

josevalim avatar Aug 31 '23 19:08 josevalim

Fantastic! 🚀

  1. When I add a second output, the first one gets duplicated. The same happens after drag and drop. Refreshing gets it back to normal. // Ah, this is because we don't prune outputs in the new LV, you don't have to worry about it, I can fix this later :)

https://github.com/livebook-dev/livebook/assets/17034772/d36417c4-8b6c-4d1b-9365-598f558b5027

  1. When popping-out the iframe is misplaced.

https://github.com/livebook-dev/livebook/assets/17034772/e900ac79-8d3e-40f5-b99e-8f9e76de4ac9

  1. Once the panel is open all markdown cells are gone from the notebook.

@josevalim I wouldn't allow markdown. It is additional complexity and since it's "output panel" allowing only outputs makes more sense (we could say rendered markdown is markdown cell's output, but I don't like blurring that boundary). I see why the rightmost placement makes sense, but at the same time putting it next to the trash icon isn't necessarily the best, because if people are using it often it's more likely they will accidentally remove cells. I'm fine placing it before the link icon to keep the alignment we've maintained so far.

jonatanklosko avatar Aug 31 '23 21:08 jonatanklosko

@jonatanklosko alright. let's leave the button where it is for now and we can bikeshed after merging. :)

josevalim avatar Sep 01 '23 06:09 josevalim

Hi @jannikbecher, just a quick check to see where we are at. :) If you want to us to wrap it up, feel free to let us know too :)

josevalim avatar Sep 12 '23 15:09 josevalim

hey @josevalim :) sorry for the late response. Today I will be able to continue the work and hopefully fix the remaining issues :+1:

jannikbecher avatar Sep 14 '23 07:09 jannikbecher

No worries, we just did the priorization for Livebook v0.11 and this feature can wait until v0.12 if you need more time. :)

josevalim avatar Sep 14 '23 07:09 josevalim