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

Support for using EditorView subclasses #18

Closed
abingham opened this issue Jan 13, 2022 · 9 comments · Fixed by #19
Closed

Support for using EditorView subclasses #18

abingham opened this issue Jan 13, 2022 · 9 comments · Fixed by #19

Comments

@abingham
Copy link

Is there any way to use a subclass of EditorView instead of EditorView itself with use-prosemirror? Looking through the code, it looks like EditorView is hardcoded, but perhaps I've missed something.

If it's not supported, would you consider supporting it? I'm not sure the best way to do it, but perhaps the class could be passed in as a property. I'd be happy to help with a PR, but I like to first make sure I do it in the way you prefer (I'm not a terribly experienced JS programmer, so I may "obvious" approaches).

@dminkovsky
Copy link
Collaborator

dminkovsky commented Jan 13, 2022

Hi @abingham. Thanks for opening this issue.

Yep, EditorView is hardcoded. It's the class that is concretely instantiated at: https://github.com/dminkovsky/use-prosemirror/blob/82d93e376e595f5784c7a584f91b759a2d43f62b/src/ProseMirror.tsx#L62-L65

I'm not a ProseMirror expert, but I'm not aware of any reason not to extend EditorView, which is not something I'd considered but seems reasonable. To support that I would:

  1. Add a type parameter V to function ProseMirror that's like function ProseMirror<V extends EditorView = EditorView>.
  2. Add the same type parameter to EditorProps such that EditorProps<V extends EditorView = EditorView>
  3. Add a prop to EditorProps<V> that is a factory that returns E.
  4. Use that factory if it was passed in the props to instantiate the editor view.

I don't have time to do this right now but if you want to do a PR I would be happy to review.

Thanks again!

@abingham
Copy link
Author

Sure, I'll try to put something together this morning. Thanks!

@abingham
Copy link
Author

Add the same type parameter to EditorProps such that EditorProps

Do you mean PropsBase rather than EditorProps?

@dminkovsky dminkovsky mentioned this issue Jan 14, 2022
@dminkovsky
Copy link
Collaborator

I started typing it out but then I realized it was easier to just edit the file :) Please see #19.

@abingham
Copy link
Author

That looks great, and quite a bit nicer than what I'd hacked together. Thanks for taking care of this. I definitely learned something!

@dminkovsky
Copy link
Collaborator

Actually, unfortunately there's a TS issue there I'm not seeing how to fix :/

@dminkovsky
Copy link
Collaborator

dminkovsky commented Jan 14, 2022

... because I was making it way too complicated :). Just pushed what I think should work for you?

@abingham
Copy link
Author

That PR seems to work well for me.

@dminkovsky
Copy link
Collaborator

Looks like it's published.

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

Successfully merging a pull request may close this issue.

2 participants