Make Excalidraw Modal render interacting user specific

[ivailop7]

Fixes #1788

Before when an Excalidraw node is added, the popup will show up for all collaborators, not just the creating user. Which would lead to a number of other issues, as shown in the Before video. Skipping the syncing for a showModal property on the ExcalidrawNode, allows for independent editing.

Before:

excalidraw_open_before.mp4

After:

excalidraw_popup_after.mp4

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 11, 2023 9:58am
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 11, 2023 9:58am

[fantactuka]

Wouldn't advice storing ui state on node itself as it might backfire. Eg here you filter it out from collab but it'll be stored in undo stack for non collab. Maybe we can come up with a better approach

[fantactuka]

Internally we did following:
Node only stores drawing state, plug-in is responsible for rendering drawings dialog and registering INSERT/UPDATE_EXCALIDRAW_COMMAND. Insert command inserts node into editor and opens dialog (isModalOpen is internal state of plugin) and sends nodeKey to dialog. When user hits save it'll use node key and update drawing state on node

[ivailop7]

Internally we did following: Node only stores drawing state, plug-in is responsible for rendering drawings dialog and registering INSERT/UPDATE_EXCALIDRAW_COMMAND. Insert command inserts node into editor and opens dialog (isModalOpen is internal state of plugin) and sends nodeKey to dialog. When user hits save it'll use node key and update drawing state on node

Gerard offered the excludedProperties as an approach, which seemed straightforward to me. Your suggestion sounds better, but I'm not sure how to skip propagation of the show modal event to the other collaborators. If you have this solved already, might be easier to just port the changes over.

[zurfyx]

Internally we did following: Node only stores drawing state, plug-in is responsible for rendering drawings dialog and registering INSERT/UPDATE_EXCALIDRAW_COMMAND. Insert command inserts node into editor and opens dialog (isModalOpen is internal state of plugin) and sends nodeKey to dialog. When user hits save it'll use node key and update drawing state on node

In this case the Node would not be the responsible to render the modal, instead the plugin would be. Admittedly, this is a much safer approach since the state is guaranteed to be offline. I wonder how either of these approaches would work with collaborative Excalidraw but this is a much more complex piece of work.

---

@fantactuka, @acywatson, I think it's worth a discussion on when to use offline properties and the tooling we want to provide around them. It's nice that we added the ability to customize what's synced over the wire but:

1. They're easy to miss because they're not self-contained into the plugin. The creator should be the plugin itself rather than expecting the user to remember adding these offline properties, a register function might do the trick but we also want to avoid coupling with an optional plugin.
2. It's hard to use them right. You have to be aware of the history and history-merge tag and the differences between collab, clipboard copy, and export JSON.

[fantactuka]

@fantactuka, @acywatson, I think it's worth a discussion on when to use offline properties and the tooling we want to provide around them. It's nice that we added the ability to customize what's synced over the wire but:

Do you recall what was initial usecase when it was added? With non-collab history and persisted content (when lexical json is stored), I wonder if it's useful or whether it can be replaced by plugin-stored state

[zurfyx]

@fantactuka, @acywatson, I think it's worth a discussion on when to use offline properties and the tooling we want to provide around them. It's nice that we added the ability to customize what's synced over the wire but:

Do you recall what was initial usecase when it was added? With non-collab history and persisted content (when lexical json is stored), I wonder if it's useful or whether it can be replaced by plugin-stored state

AFAIK the public excluded property was introduced to solve Excalidraw as well 😛 (@acywatson might be able to share more details on this first iteration)

[ivailop7]

Waiting on @fantactuka for best practice around handling local state