-
Notifications
You must be signed in to change notification settings - Fork 426
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
Proposal for Deployed Dashboards Enhancement Using Drag & Drop Functionality #2052
Conversation
Hi @jannikbecher, this is beautiful. ❤️ Curiously, this is something we had in mind for a while and I have opened an issue about this yesterday (see #2048). However, I would make it so the canvas is not only related to deployment but always visible. Imagine something like this:
It is very similar to what you did, except we want the canvas to always be running, since the canvas can be very useful during development/exploration too. |
Some additional notes:
|
I'm not sure. Given the definition of a dashboard:
I can see how one can use that feature to build other things besides dashboards. For example, this feature would be useful to help one structure the layout of their Livebook App in a non-linear notebook-like way. So maybe canvas is still a better name to catch potential use cases. Or, I don't know, maybe "free-form layout." |
Is this going in the right direction? screen-capture.3.webm
Drag & Drop not implemented yet. I'm not sure whether gridstack.js is the best library to use. Do you have any preferences? I thought gridstack would be nice, because you can write <div ... x={x_pos} y={y_pos} w={width} h={height} > and I thought it would be useful. Unfortunately it conflicts with liveview DOM updates and I couldn't figure out how to run it in parallel without using phx-update="ignore" |
Pop canvas out screen-capture.4.webmTODO:
|
Yes, fantastic!
Which is OK! maybe we should give up on it because it won't work with popped-out canvas anyway.
Maybe no library is going to work like that you should be using a phx-hook to update the entries. :) And then emit live events to update them. Some nitpicks:
|
I would keep it to make it easy to identify where the output is coming from and where it can be adjusted. |
y: non_neg_integer(), | ||
w: non_neg_integer(), | ||
h: non_neg_integer(), | ||
outputs: list(Cell.indexed_output()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't keep the outputs directly, because they wouldn't update on reevaluation and more importantly that's not something we could store in the .livemd and load later.
Because of reevaluation, it may be tricky to identify a specific output. We could store cell id + index, but if a random logger output pops up (or if the user adds a temporary inspect for debugging) it can make the index point to a wrong output. I think we should treat all outputs of specific cell atomically in this context. In most cases each rich output would come from a different cell (or it should be easy to make it so). We can store canvas placement in each cell struct and persist in cell metadata (we don't have cell ids in .livemd, so we don't want to store references to that).
And one other note, I wonder what's the best way to store the placement. Coords + size seems natural, but that is not responsive and we will need scroll when the screen is smaller than the canvas. This is an issue for deployed apps where we want things to be more responsive. Perhaps we could make the x and width quantized, so it scales with the screen, but then height cannot be a fixed value either. I'm not sure what the best solution is, just pointing out.
@josevalim if you have concerns with any of the above let me know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coords + size seems natural, but that is not responsive and we will need scroll when the screen is smaller than the canvas.
Ah, I looked at gridstack and as far as I understand x
, y
, w
, h
are already quantized and it is responsive, so ignore that part :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately I will be on a short vacation till sunday, I will change this next week :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jannikbecher there is absolutely no rush, thanks for working on this! Enjoy the vacation :D
screen-capture.5.webm
What should happen if the canvas is popped out and the user clicks on "Canvas"?
I thought it was easier to remove it from the notebook, so there aren't any duplicated id conflicts.
Maybe scroll in the notebook to the selected output in the canvas and highlight it? |
Instead of hiding it, show the button as active. If you click it again, you bring the output back. :) |
Taking a quick look around, another suitable alternative I could find is https://muuri.dev/ - it seems to be smaller in size but I am unsure which one provides the best API. I also found many other libraries but they are all unmaintained - so we may need to accept that, whatever we choose, it will likely go unmaintained, and then perhaps choose the smallest thing possible. In this scenario, draggable.js from Shopify may be an option. It is no longer maintained but it seems to be quite small. |
Heyho, managed to waste some time playing around with the Broadcast Channel API. What do you think using this for client side PubSub? I have to admit I don't fully understand the client side communication (especially the iframe part) but since the current canvas implementation uses multiple windows my naive self thinks this is a good idea :-D |
@jannikbecher I am actually not sure if we should use pubsub for this one. The canvas is shared but different users have full control if they are seeing the canvas or not. Ideally we would use the browser for communication between those parts if possible. Can that be the case? |
@jannikbecher Broadcast Channel API works across all tabs, but in general we don't want events to leak across tabs (e.g. if a cell is focused in one tab, we don't want to tell that to other tabs). I think sending messages directly between the windows is fine :) For more context, the client side pub sub is expected to work only on the current page, it's just a way to communicate between all the separate hooks. For iframes we need to use window messaging either way, because they are served from different origin. |
defp after_operation( | ||
%{assigns: %{canvas_pid: pid}} = socket, | ||
_prev_socket, | ||
{:move_output_to_canvas, _client_id, _cell_id} | ||
) | ||
when not is_nil(pid) do | ||
send(pid, {:new_data, socket.private.data.notebook.canvas_settings}) | ||
socket | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not tie the LVs together. The canvas should subscribe to data updates and work effectively as a separate client (which is very similar to what AppSessionLive does).
Heyhey, screen-capture.9.webmIn the meantime I stumbled across @zachdaniel's tweet implementing a node-based editor in elixir. I could add some collision detection and a grid for snapping and than it could completely replace gridstack.js. WDYT? |
w: non_neg_integer(), | ||
h: non_neg_integer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there even a need for resizing the output? For charts this would be really nice, but for this to happen we would need the ability to change the code cell source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the other comment, outputs generally use the available width. This is especially important for outputs that use iframes, because they live outside the main layout and we position them absolutely, so without specific width they would freak out :D
Array.from(this.grid.el.children).forEach((item) => { | ||
console.log(item.attributes); | ||
const output_id = `[id^=outputs-${item.attributes["gs-id"].value}]`; | ||
const output_el = document.querySelector(output_id); | ||
item.firstChild.appendChild(output_el); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably very bad idea to do it this way, but I couldn't come up with another solution...
I just remount the existing outputs into the gridstack items
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't put more effort into it yet, because I'm tinkering around with implementing a liveview compatible drag&drop solution
access_by_id(cell.id) | ||
], | ||
fn cell -> | ||
%{cell | output_location: nil} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add another variable to the cell, so when moving the output back to the notebook, the location gets saved.
I think it is more expected behaviour when the output moves back to the same location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, moving output back and forth is not likely and when they remove an output it's likely that the user rearranges things to fill the empty space :)
access_by_id(cell.id) | ||
], | ||
fn cell -> | ||
%{cell | output_location: %{x: 0, y: 0, w: 4, h: 2}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to figure out the size of an output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, most outputs don't have a specific size and just take whatever width they have, so we need to allocate a specific space.
@@ -10,6 +10,7 @@ defmodule Livebook.Notebook.Cell.Code do | |||
:id, | |||
:source, | |||
:outputs, | |||
:output_location, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitive needs some clearer naming
@@ -0,0 +1,141 @@ | |||
defmodule LivebookWeb.SessionLive.PopoutWindowLive do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The popped out window creates a new connection to the server. Wouldn't it be better to fetch the data from the main window? Any tips to accomplish that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is totally fine, it's the same as two users collaborating and everything is built under this assumption anyway. Pretty sure syncing everything on the client wouldn't really be viable.
|> assign( | ||
session: session, | ||
# TODO | ||
client_id: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the best way to fetch the client_id from the main window?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each process needs to register as a separate client, so we need to register this LV and it will have its own client_id.
Hey @jannikbecher thanks the updates! I talked with @josevalim and we agreed that we would get the most value if the canvas is a zoomable infinite, workspace like Count.co or Figma. I'm not yet sure if there's a library that would get us there, but that's the direction we would like to explore :) |
Okey perfect, I will look into it :) |
Actually, let's hold on this for now. Hi @jannikbecher, before we move forward, can ask you what was your original motivation for the feature? Because we have noticed we (members of the Livebook team) have two conflicting views:
Maybe those can be unified in a layer but perhaps not. We will discuss a bit more between us and follow-up. Sorry for the back and forth. |
Hey @josevalim,
TL;DR
Maybe some background if anyone is interested :) But this is not a plan or anything I try to enforce by submitting PRs. I just had some ideas and much fun hacking around with it :)
So I definitely would like to see something like the second point in livebook, but since I really like to implemented stuff in elixir/livebook as the sake of it, I'm completely open with whatever direction this will go :) |
Hi @jannikbecher, we talked about this today and we have a couple updates. Basically, a grid system will not work for us because we do not know the heights of elements up-front. For example, a text element may take certain height in some screens but wrap in others. That said, we need to go with a row-based design. You can think that each output goes to its own row:
However, I can also drag and drop 2 into 1 and split that row in two columns:
You should be able to control how much width each column has, but that doesn't need to be a concern for now. The important to keep in mind in that we don't know how much height a row takes. If 2 has fixed height and 1 is text, 2 may be a smaller than 1 in some screen and bigger in others! Once a row ends, the next row starts. Unfortunately I think this approach pretty much neglects the grid design but it should hopefully be simpler in the end. Some additional considerations:
We also won't call it canvas, most likely something like "Output grid" or "Output workspace". Internally you can call it "Output grid", we will figure out the external name later. :) |
For the name, let's go with "Output panel", unless you have better suggestions. :) |
Closing in favor of #2109 |
TODOs:
Known Bugs:
Original First Message
screen-capture.2.webm
Heyho,
It would be really nice to have the functionality to create deployed dashboards by rearranging cell outputs via Drag&Drop.
This is just a PoC and I thought it would be best to reach out to you before spending to much time with it since this will introduce a new javascript dependency and I'm not sure whether it is a good idea.
Is this something worth exploring or should I completely abandon this idea?