Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Mobiledoc 0.3.2 #62

Merged
merged 1 commit into from
Jul 16, 2019
Merged

Support Mobiledoc 0.3.2 #62

merged 1 commit into from
Jul 16, 2019

Conversation

YoranBrondsema
Copy link
Contributor

Continues work from #59.

  • Rather than rendering a e.g. style="text-align: center;" attribute on the DOM element, I'd be more in favor of simply adding a data-md-text-align="center" attribute, and leave it up to the client to implement the styling. Most likely, they will use rules like [data-md-text-align="center'] { text-align: center; } in their CSS, but at least this gives them the flexibility to implement their own styling. Additionally, it makes it easier to query for elements with a certain alignment in the DOM. Note that the Quill editor does something similar, although they add a ql-align-center CSS class on the element.
  • Fixes and adds some more tests.

Copy link
Contributor

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Let's just use setAttribute all the time.

element.dataset[name] = value;
} else {
const dataName = `data-${dasherize(name)}`;
return element.setAttribute(dataName, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

✂️ return

Suggested change
return element.setAttribute(dataName, value);
element.setAttribute(dataName, value);

Copy link
Contributor

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Approving with a small cleanup noted.

]
[MARKUP_MARKER_TYPE, openedMarkups, closedMarkups, text]
],
objectToSortedKVArray(attributes)
Copy link
Contributor

Choose a reason for hiding this comment

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

The optional section attribute value should only be in this array if the version of mobiledoc is 0.3.2, if 0.3.1 or earlier it should not be present at all. This will ensure the tests are still testing old 0.3.1 docs, which we really care about continuing to support/test.

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

Successfully merging this pull request may close these issues.

2 participants