ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.71k stars 3.24k forks source link

void blocks have space above #1226

Closed bkniffler closed 6 years ago

bkniffler commented 6 years ago

Hey, as you can see here: http://slatejs.org/#/embeds, blocks have spaces above, due to the data-slate-void > span elements that have some inline styles. If all these inline styles display: inline-block; vertical-align: top; width: 0px; height: 0px; color: transparent; are changed to only position: "absolute", things work pretty good.

ps. Why do we need these inline styles forced? Would it be possible to introduce a flag (stylingMode="className") to allow styling these via css for example`?

ianstormtaylor commented 6 years ago

Hmm weird I thought I fixed that, thanks @bkniffler I'll check this out.

bkniffler commented 6 years ago

Thanks for the quick reply. Why do we need these inline-styles in readOnly=true mode btw? If you use slate to serve google amp websites, these hurt, and I don't really see a reason for them to exist. I understand why they are needed in readOnly=false.

ianstormtaylor commented 6 years ago

Good call, for readOnly mode we shouldn't need the extra styles/spacer I believe.

I'm not sure what exactly is going on with the heights though, it seems like in index.html it has the extra space, but in /dev.html it doesn't? Are you seeing the same thing there too?

bkniffler commented 6 years ago

Got error Uncaught ReferenceError: Invalid left-hand side in assignment on /dev.html. But I'm seeing it in my local dev env with latest slate/react-slate.

ianstormtaylor commented 6 years ago

Super weird. I'm not sure what if anything is different between the dev and prod styles...

ianstormtaylor commented 6 years ago

Ah, figured it out. The dev.html file didn't declare a <!doctype html>... 🤦‍♂️

bkniffler commented 6 years ago

Thanks for the fix! But in readOnly, the data-slate-void will now contain:

<div data-slate-void="true" data-key="462">
  <div>
    <span data-key="463">
      <span data-offset-key="463-0"> </span>
    </span>
  </div>
  <div contentEditable="false">
....

and thus have a space again (only in readOnly).

Is this somehow necessary? Could we get rid of the first div?

ianstormtaylor commented 6 years ago

@bkniffler good point, dunno what I was thinking. I pushed another release that totally omits the spacer, and added some read-only rendering tests to prevent it from breaking in the future hopefully.