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

How can I use upload decorations with this library? #13

Closed
OrcaXS opened this issue Apr 16, 2021 · 7 comments
Closed

How can I use upload decorations with this library? #13

OrcaXS opened this issue Apr 16, 2021 · 7 comments
Labels

Comments

@OrcaXS
Copy link

OrcaXS commented Apr 16, 2021

Hi,

I'm trying to add Prosemirror's Upload Example to your codesandbox example.
For placeholderPlugin, adding a decoration to DecorationSet seems to be working. But placeholderPlugin.getState(state) seems to be always returning an empty DecorationSet.

Am I doing anything wrong? Should I use a different way other than the official one to implement image uploading?

Reproduction: https://codesandbox.io/s/use-prosemirror-basic-example-forked-6g2t0

Thanks.

@dminkovsky
Copy link
Collaborator

dminkovsky commented Apr 16, 2021

Hello @OrcaXS. Thank you for opening this issue and for including a reproduction.

The reason your example doesn't work is because the value state captured inside startImageUpload and then passed to findPlaceholder is the value before the decoration was added. See my fork which fixes this: https://codesandbox.io/s/use-prosemirror-basic-example-forked-funwu. I changed the dispatch function you provide to startImageUpload to return the updated value, and then use that value instead. This change makes it work.

However, it is my opinion that using ProseMirror plugins for state management is an anti-pattern with React. Why? Because React is all about top-down data flow, where state is kept up in the render tree and flows down through components as they render. ProseMirror plugins have state management because ProseMirror is intended to exist independently in environments without state management. But state management is a central component of React, so I wouldn't use plugins for state.

Instead, I recommend skipping the plugin for this use-case entirely, and using useState() (or whatever state management you use) to store an initial empty DecorationSet. Then you can update the decoration set directly in your React callbacks, and render it by passing that DecorationSet to <ProseMirror/> using the decorations prop. In my opinion this approach is much more straight forward and idiomatic. There is no using the editor state meta, no "events" ("add"/"remove"). Here is an example of this: https://codesandbox.io/s/use-prosemirror-basic-example-forked-k5v6r. This example is missing mapping transactions like in the plugin version (you'll want to do this in dispatchTransaction) but is otherwise complete.

@dminkovsky
Copy link
Collaborator

Made some edits to the reply above, sorry had lots of typos :).

@OrcaXS
Copy link
Author

OrcaXS commented Apr 17, 2021

Hi again, Thanks for your detailed explanations.

The second example doesn't clear the decoration after upload complete, here're the little changes I made:

  {...}
  const tr = stateRef.current.tr;
  tr.replaceWith(
    pos,
    pos,
    schema.nodes.image.create({ src: url })
  );
  setDecorations(decorations.remove([deco])); // remove decorations afterwards
  setState(state.apply(tr));
});


The other problem is, in Prosemirror's Upload Example, while the image is "uploading", any inserted text in the meantime won't get cleared after the "upload" is finished. However, both of your forked examples don't seem to save the changes that happens between adding and removing of the decoration. Looks like I have to capture the state just before the "upload" finishes, but I can't figure out how yet. Is achieving the same functionalities viable?

This is the behavior of the official example:

Screen.Recording.2021-04-17.at.21.13.10.mov


After all, I feel handling decoration states in React will be a better and cleaner approach. I'd try integrating those states into React meanwhile.

@dminkovsky
Copy link
Collaborator

The second example doesn't clear the decoration after upload complete

Whoops yeah I neglected to include this. Your approach is what I would have done.

The other problem is, in Prosemirror's Upload Example, while the image is "uploading", any inserted text in the meantime won't get cleared after the "upload" is finished.

Yep, that's what I meant by

This example is missing mapping transactions like in the plugin version (you'll want to do this in dispatchTransaction) but is otherwise complete.

In the PM upload example that is done with

      // Adjust decoration positions to changes made by the transaction
      set = set.map(tr.mapping, tr.doc)

So you'll want to do this too. To do this, use the dispatchTransaction prop on <ProseMirror /> instead of onChange, and every time you get a transaction from the editor:

  1. apply it to the current state and update the state,
  2. use it to map the decoration set, like in the example.

I don't have time to update the example w/ this right now, but I do this in my project and it works well. Let me know if you can't get it working.

@dminkovsky dminkovsky changed the title Plugin.getState() not returning latest plugin state How can I use decorations with this library? Apr 17, 2021
@dminkovsky
Copy link
Collaborator

dminkovsky commented Apr 17, 2021

BTW I updated the title of this issue, so that anyone facing this problem can more easily find it. I hope you don't mind. Feel free to further improve or revert it.

@dminkovsky dminkovsky changed the title How can I use decorations with this library? How can I use upload decorations with this library? Apr 17, 2021
@OrcaXS
Copy link
Author

OrcaXS commented Apr 18, 2021

Thanks for explaining that decoration mapping part, looks like I wasn't understanding enough about the official example...
Here's the new fork I made:
https://codesandbox.io/s/use-prosemirror-basic-example-forked-mt1jd

Using stateRef.current.tr and setting a new decoration set with mapping in dispatchTransaction, it's working like the official one apparently.

This part:

  const decorationsRef = useRef(decorations);
  decorationsRef.current = decorations;
  // ...
  const stateRef = useRef(state);
  stateRef.current = state;

looked a little bit confusing, though. useRef API says:

useRef returns a mutable ref object whose .current property is initialized to the passed argument (initialValue).

But in this case, I guess we have to ensure every decorationsRef and stateRef does hold the latest state/decorations after state updating happens? I'm not pretty sure about this.

If this is the recommended way for implementing uploading decorations, can we update the original example to help anyone run into the same problem? Cleanups and adding necessary comments may be needed.

@stale
Copy link

stale bot commented May 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 18, 2021
@stale stale bot closed this as completed May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants