bustle/mobiledoc-kit

[WIP] Centering / section justification tracking issue

Closed this issue ยท 13 comments

The main chunks of work necessary for this are as follows:

  • Update the format itself to allow section attributes (we can handle those _section (baseclass))
    • This will be done similarly to how markups handle attributes (for example: PostNodeBuilder#createMarkup)
      • We will create one valid section attribute of data-md-text-align. The data-md- namespace for attributes will be where we create any future section attributes. Like with markers arbitrary attributes (like class, style, etc) are not part of the spec though it seems like that has not stopped anyone from adding them.
  • Update the editor renderer to handle section attributes
    • This will be handled very similarly to how markups are currently handled in editor renderer (for example EditorDOM#createElementFromMarkup)
      We'll likely need to add something to the EditorDom to wrap sections in an element with a style tag for centering et, al
  • Update DOMRenderer to handle section attributes
  • Update TextRenderer to not fail when attributes are present on section. This is mostly just to prevent errors when MD format changes since we don't/can't do anything with section style information in text rendering (basically this is just a noop for section attributes)

I love this proposal โ€“ I certainly would need it in the not so far future ๐Ÿ˜…

Like with markers arbitrary attributes [..]

I have two questions: currently the Markers only allow href and rel attributes (source), so it is not really possible to create markup with custom attributes.

Would it be possible to remove this restriction? And what are your thoughts about non-string attributes (that are not rendered in the default editor renderer, but in a custom one)?

@apfelbox I'm not sure why those constraints are in place except to somewhat defensively code such that it forces to follow markup spec. Someone else might be able to shed some light on why the code is as it is and we can pursue allowing diff attributes via some configuration.

@apfelbox the restriction on attributes ensures that the mobiledoc document format itself is portable. For example if we document data-md-text-align as having four valid values (left, right, center, justify) we can obviously map that to CSS either through styles or classes+CSS. Anyone writing a renderer for mobiledoc that is not outputting HTML, for example rendering in a native mobile app, can also reproduce those styles. Even in the land of HTML this abstraction is nice: For example in email you may want to use a different method to style this content than in the browser.

If we permit arbitrary attributes we will empower Mobiledoc users to create their own bespoke versions of Mobiledoc which can't benefit from shared tooling, shared rendering, and interoperability (full fidelity copy/paste for exmaple).

Now if you don't care about of that stuff and you really insist, The whitelists for attributes/elements are all just arrays in JS. I know some people import those and add bespoke attributes for example. Just keep in mind that by doing so you've effectively opted in to a bespoke, unique document format. A goal of Mobiledoc was to avoid that very end point.

The alternative is that you think about how your bespoke change can be generalized into a part of the Mobiledoc format and official editor/renderer. That is exactly what is being done in this ticket. You could already hack this via private API, now we're making it public/documented API.

tl;dr I don't expect Mobiledoc will every allow arbitrary tags or attributes because that would be at odds with our baseline goals for interoperability and common tools. If you want to break with that for now you can hack it through private APIs. We won't go out of our way to block people doing this.

WIP PR to implement above: #670

Hi @mixonic,

thank you for your long and detailed explanation, I really appreciate it! I understand you viewpoint and share it.

However I would like to show you my use case and where I am coming from:
We are using mobiledoc(-kit) as base for our rich text editor in our CMS. We can add things like image URLs and especially links just as plain text. In the case of internal links on our own pages, we would however like to associate the ID with it โ€“ to update the link dynamically as soon as the page URL changes. We could build a "fake URL" that we parse on rendering and rewrite dynamically โ€“ but that feels plain wrong.
The same goes for giving the option to open links in new tabs. Right now the attribute "target" is forbidden, so that it is currently not possible to add it to an URL.

Again โ€“ I completely understand your motivations, but having to import a global array (and trusting the module bundler that all imports are singletons โ€“ which isn't guaranteed by anything) and modifying it... seems weird. Maybe you could move the check from the Markup constructor to postBuilder.createMarkup() and we could add a separate postBuilder.createCustomMarkup() or something similar?

Right now we also have to use horrible hacks, that I would like to get rid of, but I am not really comfortable with exchanging one hack with another sort-of hack.

You can already create atoms and cards that aren't portable โ€“ in this case it seems fine to have "black box content elements" โ€“ but with markup it doesn't?

I understand if you decide that this is out of scope / explicitly not desired, but just wanted to provide you with our use case. Thanks anyway again, though ๐Ÿ˜Š

Upon initial consideration I believe the text renderer can safely remain unchanged since the presence or absence of the attributes in the section won't affect it.

Marking as completed, unless someone has a better insight than me. ๐Ÿป

I think this is ready for a more thorough review @mixonic (if/when you have the time) ๐Ÿ˜„

@rondale-sc how's this one coming along?

@ZeeJab This and the other review here is on me! I will take a deep look early this week.

@mixonic ๐Ÿ‘‹ ๐Ÿ‘‹ ๐Ÿ˜„

@mixonic hi ๐Ÿ‘‹

@mixonic ๐Ÿ˜…

I've been getting caught up on the work for this. The main WIP PRs are:

These need to be cleaned up to bump the Mobiledoc spec implementation instead of modifying the 0.3.1 spec. I'll be doing this.