Slate: code-highlighting example fix

Created on 25 Oct 2018  Â·  4Comments  Â·  Source: ianstormtaylor/slate

1.https://www.slatejs.org/#/code-highlighting

2.Two carriage returns

image

bug ♥ help

Most helpful comment

I think I found a bit of a better solution for this. As best I can tell, decorations themselves never get normalized. This is why we end up with null values on the points even though they should be getting normalized away.

There are two entry points for creating decorations setDecorations() and decorateNode(). The approach I'm thinking is that Decoration should have a normalization method that looks like:

normalize(node) {
  return this.merge({
    anchor: this.anchor.normalize(node),
    focus: this.focus.normalize(node),
    mark: this.mark.normalize(node),
  })
}

And then we should normalize when we have access to a node. So in getDecorations(), we normalize before they get used anywhere:

return decorations.map(dec => dec.normalize(this))

We need to do it here, because getDecorations() gets called when the document is being rendered already, and we can't wait for a top level normalization. This seems to work pretty well locally but I havne't done extensive testing. I'm most worried about performance.

We also need to normalize at the top level in case someone passes decorations in directly through something like setDecorations(). I think this needs to happen in Editor#normalize() along with the document normalization, but still wrapping my head around how this would work. Probably needs to happen after the document finishes normalizing, and just another map to normalization like the example above.

@ianstormtaylor What do you think?

All 4 comments

I think I found a bit of a better solution for this. As best I can tell, decorations themselves never get normalized. This is why we end up with null values on the points even though they should be getting normalized away.

There are two entry points for creating decorations setDecorations() and decorateNode(). The approach I'm thinking is that Decoration should have a normalization method that looks like:

normalize(node) {
  return this.merge({
    anchor: this.anchor.normalize(node),
    focus: this.focus.normalize(node),
    mark: this.mark.normalize(node),
  })
}

And then we should normalize when we have access to a node. So in getDecorations(), we normalize before they get used anywhere:

return decorations.map(dec => dec.normalize(this))

We need to do it here, because getDecorations() gets called when the document is being rendered already, and we can't wait for a top level normalization. This seems to work pretty well locally but I havne't done extensive testing. I'm most worried about performance.

We also need to normalize at the top level in case someone passes decorations in directly through something like setDecorations(). I think this needs to happen in Editor#normalize() along with the document normalization, but still wrapping my head around how this would work. Probably needs to happen after the document finishes normalizing, and just another map to normalization like the example above.

@ianstormtaylor What do you think?

This is by far the most common error coming out of our Slate editor… @ericedem still thinking of taking on this fix?

@tommoor Ahh sorry, this one fell off my radar. This is the branch I was working out of is here if you want to take a look at the code for your own PR. I haven't been working on Slate much recently but will probably be digging back in in the next few weeks and might be able to tackle this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chriserickson picture chriserickson  Â·  3Comments

JSH3R0 picture JSH3R0  Â·  3Comments

ianstormtaylor picture ianstormtaylor  Â·  3Comments

ianstormtaylor picture ianstormtaylor  Â·  3Comments

gorillatron picture gorillatron  Â·  3Comments