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

Allow custom editable value renderers #454

Closed
VincentK-ZA opened this issue Jun 24, 2024 · 7 comments
Closed

Allow custom editable value renderers #454

VincentK-ZA opened this issue Jun 24, 2024 · 7 comments

Comments

@VincentK-ZA
Copy link

Looking at the custom value renderer examples, none of them really take over the the editing rendering.

I've been working on getting this work, but from I could see, I had to fork the project in order to get it working.

Here's an example of a custom editable div in our project, using codemirror:
custom-editable-div

Please see this PR (internal to the forked repo for now) for what I believe would need to be changed.
This includes an EditableCodeMirror component for demo purposes on the examples page.
Our actual implementation relies on getContext to get actual completion/suggestion data, since I didn't want to pass any more props through EditableValue.

Let me know what you think about this proposal, and if you'd be open to merging a change like this into the project.

There are some tricky things regarding focus/mousedown events that I have not managed to solve as of yet.
For example, I couldn't get the suggestions panel to stay open if the user tries to drag the scrollbar to see more suggestions.

@josdejong
Copy link
Owner

Thanks for your inputs Vincent.

Can you explain what you think is missing in API in order to (easily) create an editable value component yourself?

@VincentK-ZA
Copy link
Author

Hi Jos

Thanks for your reply and for the examples.

In our case, we are trying to make a custom editable input that functions and looks almost identical to the default editable div, just with autocompletions added.

So there are some internal/not exported utility functions that would be nice to reuse as is.
Namely: getFocusPath, getValueClass, isObjectOrArray, keyComboFromEvent we are currently relying on, and they are not currently exported it seems.

Lastly, there's the case of some internal logic checking specifically for a div and a jse-editable-div class being set.
However this isn't really an issue, as in our case we do have a div element and we can set the needed class, in order to hook into the same functionality.

@josdejong
Copy link
Owner

It should be as easy as possible to to extend and customize the editor, so this is really helpful!

In #461 I've exported more utility functions. Would that solve your issue?

We can address the magic internal logic via #460. I like getting rid of this special code. Note that there the code uses an internal function expandSmart(), new in the upcoming v1. This function uses the internal documentState. It's a separate topic to make an API to allow control this documentState containing which nodes are expanded, but you can still build something similar using the public method .expand().

@VincentK-ZA
Copy link
Author

VincentK-ZA commented Jul 10, 2024

Hi Jos

Thanks for the quick work on this, those two PR's look good, just on the first one I added a comment to also export the stringConvert utility function.

Removing the internal magic and using a callback seems like a better approach for sure. For what it's worth, we only really have the cancel function hooked up, and aren't relying on refresh at all (It's just a () => {} noop for now). Haven't noticed any issues.

Looking forward to v1

@josdejong
Copy link
Owner

Thanks for checking out the PR's!

@josdejong
Copy link
Owner

The just published v1.0.0 now exports more utility functions (see #461), and it doesn't rely anymore on the special internal logic (see #460).

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

No branches or pull requests

2 participants