Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Preserve empty inline nodes #167

Open
Goaman opened this issue May 12, 2020 · 6 comments
Open

Preserve empty inline nodes #167

Goaman opened this issue May 12, 2020 · 6 comments
Assignees

Comments

@Goaman
Copy link
Contributor

Goaman commented May 12, 2020

In html when I parse an inline node (e.g. span, b, i) that has not text inside:

<b class="myClass"></b>

It disappear in the vdocument.

We should keep these nodes as they might represent something in the context they are used. For instance, they can have content in css:

.myClass::before {content: 'foo'}

Maybe we could handle that case with an EmptyNode that is created when no char text is found?

@Zynton
Copy link
Contributor

Zynton commented May 15, 2020

That's right indeed. So far only the ones with a font awesome class will be parsed but it would be nice to have some ad-hoc system for that.
Just some random thoughts to spark discussion.

  • While it means something in HTML, it doesn't for instance in Markdown.
  • I'm guessing it would be an AtomicNode.
  • FontawesomeNode should probably not depend on it as they would both be plugins, but then we'd have duplicated logic (the invisible spaces for the range for instance).
  • In these cases the HTML tag doesn't actually mean anything.
  • What we need to render a Format is an AtomicNode. Now these would be Formats parsed as if they were AtomicNodes.
  • Regarding the two above comments, I guess we could parse them as Formats but that in general Formats are applied to EmptyNode when they have no children in the DOM. But then EmptyNode should be in core.

@dmo-odoo

@dmo-odoo
Copy link
Collaborator

dmo-odoo commented May 15, 2020

  • While it means something in HTML, it doesn't for instance in Markdown.

Does it change something ? I mean, we could still parse it, have an empty node "storing" the parsed value, and if it truly does not have any effect for this type of content then the rendering will ignore it I guess or render it just as it was parsed and the effect will be the same anyway ?

  • I'm guessing it would be an AtomicNode.

Probably, yes.

  • FontawesomeNode should probably not depend on it as they would both be plugins, but then we'd have duplicated logic (the invisible spaces for the range for instance).

I don't understand why this comes up in the discussion. Storing empty inline node classes is a generic issue, not realted to FontAwesome. It just happens that the FontAwesome plugin already implements a similar albeit limited form of this. I'm not sure we would need the invisible spaces for the range in the generic issue, which is basically "don't loose stuff by parsing them in our abstraction".

  • In these cases the HTML tag doesn't actually mean anything.

Sure, but that's not a reason to loose it, is it ?

  • What we need to render a Format is an AtomicNode. Now these would be Formats parsed as if they were AtomicNodes.

That's a current limitation. Ideally, modifiers would be rendered independently for every node.

  • Regarding the two above comments, I guess we could parse them as Formats but that in general Formats are applied to EmptyNode when they have no children in the DOM. But then EmptyNode should be in core.

AtomicNode is in core and Modifier will soon be as well.

@Goaman
Copy link
Contributor Author

Goaman commented May 15, 2020

I agree with @dmo-odoo.

While it means something in HTML, it doesn't for instance in Markdown.

In these cases the HTML tag doesn't actually mean anything.

Is it important to systematically have correspondance? In html <div> tags that are present for layout purpose have no meaning in markdown.

@Zynton
Copy link
Contributor

Zynton commented May 15, 2020

Those were just thoughts hey guys, not objections ;-) Things that I thought deserved some attention.

I also don't think we should lose the html tag but I do think it's kind of strange to have it all over the place while html is a plugin ^^

AtomicNode is in core and Modifier will soon be as well.
So you want EmptyNode in core too then?

@dmo-odoo
Copy link
Collaborator

dmo-odoo commented May 15, 2020

I don't want EmptyNode. I expect AtomicNode to be enough.

@dmo-odoo
Copy link
Collaborator

The span case of this issue has been fixed with 1a9fc5e.

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

No branches or pull requests

3 participants