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 nodes to be conditionally void #2538

Closed
brendancarney opened this issue Jan 16, 2019 · 3 comments
Closed

Allow nodes to be conditionally void #2538

brendancarney opened this issue Jan 16, 2019 · 3 comments

Comments

@brendancarney
Copy link
Collaborator

Do you want to request a feature or report a bug?

Feature

What's the current behavior?

Nodes are statically void depending on the schema.

Proposal
Add a way to allow nodes to be conditionally void, determinable by some factor outside of Slate's document structure.

Use Case
One use case I have is an editable HTML node inside the editor. This node has two modes: edit and preview. In edit mode, the editor behaves as it normally would. In preview mode, the node should function like a void node.

Possible Solution
This solution was suggested by in Slack by @isubasti, with input from @alanctkc: https://slate-js.slack.com/archives/C1RH7AXSS/p1547614932631900

We could make isVoid in the schema allow a function as a value. This function could be passed editor and node as arguments. In my case, I would provide the editor a prop that indicates which HTML nodes are in which modes, and read from that prop to determine whether or not the node is void. Example schema:

{ 
  blocks: {
    html: {
      isVoid: (node, editor) => editor.props.htmlNodes[node.key].state == "preview"
    }
  }
}

I'd be happy to help implement something like this, and would also be happy to help with any other solution to this problem. Thanks!

@alanchrt
Copy link
Contributor

@brendancarney A side thought about this solution for your specific use case-- one thing I've run into is that editor.props is useful for carrying data/state around places throughout the editor, but you still need a way to trigger some recomputation using the prop values after a change change.

In your case, if isVoid gets implemented with an optional function like this, it still won't be computed until that node is normalized. If the node itself doesn't change when you switch in/out of edit mode, you'll need some manual way to normalize it again (and recompute isVoid) when you change the state.

But, to address the change overall, I think it's a great addition. It would make sense, in my opinion, to update the rules that got similar treatment in #2151 so they take (node, editor) arguments as well (for consistency.

So:

type: type => type !== 'list'

Becomes:

type: node => node.type !== 'list'

This is also consistent with normalizeNode and decorateNode.

@ValeriiMal
Copy link

ValeriiMal commented Apr 18, 2019

Hi there!

@alanctkc I'd like to add one more use case where I need this dynamically set isVoid property.

How it works now:
Editor in my app adjusted to render a list of notes. Each note is a list item. I have a toolbar button that converts text note to an HTML card component that should not be editable, only focusable. Now, I need to create new editor value object with new node types in order to make particular nodes as void because the only way to make node void is to use a schema that depending on the node type can decide is that node void or no.

Problem:
adding new node subtype (e.g. 'NOTE' and for HTML note it will be 'NOTE_VOID') just to set it as void makes me add more code to create the whole value object. This leads as well to unfocusing the editor which is not good for user experience.

Preferable solution:
I'd like to use Operation set_node with property field as { isVoid: true } to dynamically set some node to void mode.

@ianstormtaylor ianstormtaylor mentioned this issue Nov 6, 2019
@ianstormtaylor
Copy link
Owner

Fixed by #3093.

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

Successfully merging a pull request may close this issue.

4 participants