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

Adding a shared sample post content #178

Merged
merged 3 commits into from
Mar 3, 2017
Merged

Conversation

youknowriad
Copy link
Contributor

Adding a sample post content to be used by the two technical prototypes.

@youknowriad youknowriad added the [Type] Enhancement A suggestion for improvement. label Mar 3, 2017
@youknowriad youknowriad self-assigned this Mar 3, 2017
@youknowriad youknowriad requested a review from mtias March 3, 2017 17:02
@aduth
Copy link
Member

aduth commented Mar 3, 2017

A few observations for how the single instance prototype will have trouble supporting this:

  • The single instance prototype doesn't currently use the comment syntax. Instead, it relies on data- attributes.
  • The single instance prototype's concept of a block does not currently extend to text "elements" like paragraphs or even headings. Instead, those are expressed as the raw DOM nodes (p, h2).

I think both of these worries could be overcome without too much trouble.

To the text itself:

  • We should probably move the mountains image to a secure domain, so as not to hog bandwidth from @jasmussen's personal site and to avoid mixed content warnings.
  • The mountains image is very large. The style for full-bleed is an interesting approach, and it seems as though it should work if we move to supporting IE11+. Would be good to see if it's well supported in a typical theme.
  • To the above point, maybe we ought to assign these as classes so we can vary the implementation as needed. For example, some of my more recent prototyping has been in TinyMCE's frame mode, which would hide the overflow from the large image. Similarly, it would be nice to migrate away from text alignment assigned via inline style attributes.
  • The second right-aligned paragraph appears to have broken realignment in the TinyMCE-per-block prototype.
  • Should we keep the large heading at the top of the text? ("1.0 Is The Loneliest Number")

All this being said, I think it's good to work from a common base. I'd be fine with merging this more-or-less as-is, perhaps with consideration of a few of the above points on the text, then we can continue to iterate and work through the challenges highlighted above.

@aduth
Copy link
Member

aduth commented Mar 3, 2017

To address a few of the points above, I simplified the markup (removed aligned paragraph, full-bleed image) and added a "complex" example: an oEmbed link. This one seems like it'll be challenging to support in a static mockup, but I figure we can keep it for now as an ideal. The TinyMCE-per-block prototype will ignore the unsupported type anyways. Updates also include a link in the quote citation, which I think we should support (that's how it's shown in the original text). It's not linked yet in the prototype, but 9473413 at least avoids the HTML from being shown as escaped.

I also reformatted the content into a joined array of strings to make it slightly easier to maintain since I'll assume we'll want to make future revisions.

@aduth
Copy link
Member

aduth commented Mar 3, 2017

I also reformatted the content into a joined array of strings to make it slightly easier to maintain since I'll assume we'll want to make future revisions.

I also observed that, when attempting to .join( '\n' ), the TinyMCE-per-block prototype (or more likely the parser) didn't handle the newlines very well. For backwards compatibility, we should expect that many post_content fields will have newlines, so at some point we should plan to look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants