Skip to content
This repository was archived by the owner on Nov 18, 2017. It is now read-only.

Switch to blocks/inlines from marks - #4

Open
mxstbr wants to merge 2 commits into
masterfrom
switch-to-blocks
Open

Switch to blocks/inlines from marks#4
mxstbr wants to merge 2 commits into
masterfrom
switch-to-blocks

Conversation

@mxstbr

@mxstbr mxstbr commented May 7, 2017

Copy link
Copy Markdown
Contributor

This PR switches mentions to be blocks. This is necessary to be able to position the suggestion portal right next to the mention.

The problem with this PR is that getting this to work correctly requires us to override the line and default nodes to be spans, which is obviously not optimal:tm: as it'll break a bunch of folks editors.

I tried using setInline/splitInline/... instead of setBlock/... but for some reason that didn't do anything. Maybe @ianstormtaylor has an idea why this exact change with xInline rather than xBlock doesn't work?

/cc @oyeanuj

Comment thread src/index.js
),
line: ({ attributes, children }) => (
<span {...attributes}>{children}</span>
),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part shouldn't be necessary if we got inlines to work for mentions.

@mxstbr
mxstbr force-pushed the switch-to-blocks branch from ae2331b to 01442d2 Compare May 7, 2017 09:44
@oyeanuj

oyeanuj commented May 7, 2017

Copy link
Copy Markdown

I feel like inline feels more natural to me, other than marks. @ianstormtaylor posted an example of how he was using both inline and the Portal here - ianstormtaylor/slate#644 (comment)

If that doesn't help, I'll be happy to take a look at why setInline might not be working.

@mxstbr

mxstbr commented May 7, 2017

Copy link
Copy Markdown
Contributor Author

Please do take a look, I've unsuccessfully tried to make this work for too long! I must be missing something minor, but this feels like an Inline rather than a block for sure.

@oyeanuj oyeanuj changed the title Switch to blocks from marks Switch to blocks/inlines from marks May 7, 2017
@oyeanuj

oyeanuj commented May 7, 2017

Copy link
Copy Markdown

Here is how far I reached: https://github.com/oyeanuj/slate-mentions/tree/switch-to-blocks

I see some issues with insertInline for which I have noted here: ianstormtaylor/slate#804

It also seems like just wrapping inline, and then moving it back to start of selection is leading to an error:

If I use wrapInline.move(1), I get an error: Cannot read property 'firstChild' of null at findDeepestNode (find-deepest-node.js:15)

But, its a start..

@oyeanuj oyeanuj mentioned this pull request May 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants