Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Default pipeline should include <section/>s #279

Closed
Tracked by #6
davidnuescheler opened this issue Apr 23, 2019 · 26 comments · Fixed by #379 or adobe/helix-cli#1037
Closed
Tracked by #6

Default pipeline should include <section/>s #279

davidnuescheler opened this issue Apr 23, 2019 · 26 comments · Fixed by #379 or adobe/helix-cli#1037
Assignees
Labels
enhancement New feature or request released

Comments

@davidnuescheler
Copy link
Contributor

currently the default pipeline renders thematic breaks (---) in markdown to <hr> and while it is a departure from githubs rendering they probably should correctly translate to <section> ... </section> which definitely is a lot more useful for DOM query selectors.

@trieloff trieloff added the bug Something isn't working label Apr 24, 2019
@trieloff
Copy link
Contributor

@koraa do you remember changing this?

@kptdobe kptdobe added enhancement New feature or request and removed bug Something isn't working labels Apr 24, 2019
@kptdobe
Copy link
Contributor

kptdobe commented Apr 24, 2019

It is not a regression, it is an "evolution". The default document has always been separated from the sections handling: it has always contained the "raw" html without extra processing. The ask here is to at least generate the <section> tags into the default document.

@koraa
Copy link
Contributor

koraa commented Apr 24, 2019

@trieloff What do you mean?
@davidnuescheler See #260 :)

@trieloff trieloff changed the title Default pipeline should include <section/>'s Default pipeline should include <section/>s Apr 24, 2019
@trieloff
Copy link
Contributor

I don't remember ever seeing <hr> in the DOM, so I wondered if this might be a regression, but it looks like the fix (i.e. #260) is what's missing.

@kptdobe
Copy link
Contributor

kptdobe commented Apr 24, 2019

I am almost sure that the code that handles the sections never touched the original content document and I have seen the <hr>. sections have always been a separate concept, if you want to use them, you need to iterate and output each section.
The ask here is that context.content.document contains the <section> tags instead of the <hr> (and the correct children).

@trieloff
Copy link
Contributor

Well, then it is an enhancement indeed.

@koraa
Copy link
Contributor

koraa commented Apr 24, 2019

@trieloff When I test this on the current pipeline,


is rendered as just that – hr. I think we just never really did use
much in our examples :)
AFAICS, the current behaviour is not really a bug – the way sections where supposed to be used until now was just to use the content.sections property and manually convert to html. This is indeed what #260 is intended to change :)

@trieloff
Copy link
Contributor

Good. But I agree with @kptdobe that it would make sense to split write-protection removal and section handling into two separate PRs.

@kptdobe
Copy link
Contributor

kptdobe commented Apr 24, 2019

ah ah, I think we all agree then ;)
We just reached the point where we think sections are a good thing and should now be part of the "default".
Note that if there is no thematic break in the document, you will end up with one single section. That's the current behavior, I think it makes sense to keep it but it can be debated. Question might be raised at one point.

@kptdobe
Copy link
Contributor

kptdobe commented Apr 24, 2019

Note also that #143 is a slightly different thing which will be invalidated by the current issue: idea here was to have a VDOM object for each section which we will get for free once the sections are in the content.document object. I am going to close #143 and use this one as a replacement. Or maybe: if we keep the sections object, we can add a document property that is a reference to the DOM object inside the content.document object.

@trieloff
Copy link
Contributor

Note that if there is no thematic break in the document, you will end up with one single section. That's the current behavior, I think it makes sense to keep it

Agree.

@davidnuescheler
Copy link
Contributor Author

Note that if there is no thematic break in the document, you will end up with one single section. That's the current behavior, I think it makes sense to keep it

I agree that that's ok, however i don't find it particularly intuitive. Following our "intuitive over consistent" principle, I would vote for no sections, if there are no thematic breaks.

@trieloff
Copy link
Contributor

trieloff commented May 2, 2019

I don't think that would make it easier to use, because you'd always have to check if content.sections exists and has a length > 0.

@davidnuescheler
Copy link
Contributor Author

The sections are definitely not intuitively expected in DOM from an md that has no ThematicBreaks. Also, playing with the sections I found them quite annoying in some ESI usecases I had, that would just include let’s say a short header or footer...
Also a superfluous section inside a header or footer feels semantically wrong in html..

@ramboz
Copy link
Contributor

ramboz commented May 3, 2019

@kptdobe, @davidnuescheler: Also wonder how this relates to #86. I was about to look into that one, but wonder if it still applies if we don't want any sectioning when there are no thematic breaks.

Since the HTML5 spec introduces implicit sectioning (see: #86 (comment)), I also wonder what added value the thematic breaks bring if we explicitly introduce sections for all headings.

With the folling examples:

example.md

# Foo
## Bar
### Baz
## Qux

Assuming the pipeline performs explicit sectioning, the former example would result in the following HTML:

<section>
  <h1>Foo</h1>
  <section>
    <h2>Bar</h2>
    <section>
      <h3>Baz</h3>
    </section>
  </section>
  <section>
    <h2>Qux</h2>
  </section>
</section>

which is what the HTML5 document outline would be anyway.

Now, assuming we use thematic breaks for explicit sectioning instead, then it seems to me it is easy to break the intuitive outline:

# Foo
## Bar

---

### Baz
## Qux

would turn into:

<section>
  <h1>Foo</h1>
  <h2>Bar</h2>
</section>
<section>
  <h3>Baz</h3>
  <h2>Qux</h2>
</section>

The HTML5 outline would even be wrong then, as it would have:

<section>
  <h1>Foo</h1>
  <section>
    <h2>Bar</h2>
  </section>
</section>
<section>
  <section>
    <h3>Baz</h3>
  </section>
  <h2>Qux</h2>
</section>

Can we assume all authors will consciously section the content as needed, or do we want the pipeline to enforce proper sectioning?

Another point to consider is that the HTML5 sections always have a start and an end tag which lets you nest them as needed, while the thematic breaks we have only apply on the 1st level as we only have a "start" tag. On the other hand, introducing explicit sectioning on all headings is an opinionated decision, and we may not want to enforce that decision for helix.

@trieloff
Copy link
Contributor

trieloff commented May 6, 2019

The HTML implicit sections are different from the explicit sections you'd put in your markdown, this is also why I'm not a fan of #86.

@koraa
Copy link
Contributor

koraa commented May 6, 2019

The implicit sections are a different kind of sections. We are talking mostly about the <section> tag; the sections in the specs you cited concern how html outlines/overviews of pages are formed.
The section tag is mostly useful in our case as a css styling feature.
Also the spec you cited is a draft from 2011 which has never been implemented anywheree; since it's from 2011 I think the chances of it ever being a proper standard are slim.

@ramboz
Copy link
Contributor

ramboz commented May 6, 2019

@trieloff I do understand they are different, I'm just trying to understand how they are supposed to work together and how to avoid unexpected behaviors if you mix them.

chances of it ever being a proper standard are slim

@koraa Could be indeed, although I've also seen specs in draft for 10+ years at the W3C before they made it into a standard. So you never know.

Anyway, I'm just trying to get my head around how an author would really use these and what the expectations would be. I get the easy trivial cases, but I'm more interested in the edge cases and more complex examples and also how they relate to the content structure and accessibility of the document.

I can definitely create a markdown like:

# Foo
## Bar
Lorem ipsum dolor sit amet

---

### Baz
consectetur adipiscing elit

---

## Qux

to get an html similar to:

<section>
  <h1>Foo</h1>
  <h2>Bar</h2>
  <p>Lorem ipsum dolor sit amet</p>
</section>
<section>
  <h3>Bar</h3>
  <p>consectetur adipiscing elit</p>
</section>
<section>
  <h2>Qux</h2>
</section>

and do a 3 column layout with CSS based on the sections, but it would be incorrect from an accessibility point of view as the sections are exposed in the browser API. The document outline would be off.

As a simple author editing that page, I may not be aware of all the WCAG constraints for instance and end up breaking the accessibility of my website's pages just because I'm trying to make the document easier to read on a widescreen.

If we are only using thematic breaks for CSS work, then I'd argue that these shouldn't be <section> tags in the 1st place (as the tag has a semantic meaning), and should probably just be plain <div> instead.

@trieloff
Copy link
Contributor

trieloff commented May 6, 2019

In Helix, we use sections to outline sections of a document. A section is a logically consistent part of a document. Use cases for sections in Helix include:

  • content-aware presentation (a section can detect if it is a gallery or a hero, or a longer textual tract)
  • content targeted for specific audiences or use cases (conditional content)
  • variations of content for the purpose of A/B testing

The use of thematic breaks and in-document YAML blocks is a choice we've made to highlight sections with decent markdown editor support over the introduction of custom HTML elements.

The use of the section HTML element in output is consistent with the semantic meaning of a section in Helix and can be overridden by developers, if so desired.

@ramboz
Copy link
Contributor

ramboz commented May 6, 2019

@trieloff

In Helix, we use sections to outline sections of a document.

I may be misunderstanding something, but that's exactly the point I'm challenging here. My point is the examples I've seen so far are mostly representation decisions rather than actual semantic blocks. The <section/> is used as a wrapper for styling or JS targeting, and not to expose a semantic meaningful block to the document outline.

The rule of thumb here is:

  • if you introduce --- to be able to target the block via CSS or JS, then it's probably not a semantic <section/> and just a wrapper <div/>
  • if you do it to expose a new section in the document outline with a heading that you can link to using #my-header, then it's probably a semantic <section/> and not just a wrapping <div/>

Taking the 3 examples you give:

content-aware presentation
Whether you represent your section as gallery, or hero, or list, or multiple <p/>, does not change the content it has, nor the information provided to the "disabled" user or the document outline. You also do not index them differently for SEO purposes. It's just a design choice. If you needed to provide semantic variations you'd also work on using <main/> vs. <article/> vs. <section/> vs. <aside/> for instance.

content targeted for specific audiences or use cases
I'm not sure I see a clear use case here. If I had to guess one, I'd go with a graph that needs to be represented in a different way depending on the audience.

Let's take blind vs. color blind vs. person with no disability vs. mobile.

The blind person will want a textual voiceover description of the graph, a color blind person will want a grayscale version of the graph (or yellow-based), the non-disabled person will use the default one, and the mobile consumer will want, say, a vertical version of the graph.
The semantic will be the same for all, a <figure/> tag with a <figcaption/>, but the actual content used will be different based on the audience (<img longdesc="…"/> vs. <img src="grayscale.png"/> vs. <img src="default.png"/> vs. <img src="mobile.png"/>.

variations of content for the purpose of A/B testing
Again, semantics will be the same for each version, just the content would be different. The fact that you render <img/>+<p/>+<button/> centered in A while you may be using <p/>+<img/>+<button/>+<p/> aligned right in B still is just a design decision that does not change the semantic block or the document outline.

I feel the fact that it sometimes works out when using <section/> is just an edge case when playing with simple examples, rather than the norm here, and enforcing <section/> tags ends up being an opinionated decision in the framework that can impact both a11y and SEO in undesired ways.

I would be careful with this and rather opt-in than opt-out. I'm not fully sure how the YAML blocks are used at the moment, but I could imagine having something like:

---
class: hero
tag: section
---

# Foo

Lorem ipsum dolor sit amet

[CTA](/register)

vs.

---
class: list
---

# Foo

Lorem ipsum dolor sit amet

[CTA](/register)

That respectively render something like:

<section class="hero">
  <h1>Foo</h1>
  <p>Lorem ipsum dolor sit amet</p>
  <p><a href="/register">CTA</a></p>
</section>

(rendered as block elements centered)

vs.

<div class="list">
  <h1>Foo</h1>
  <p>Lorem ipsum dolor sit amet</p>
  <p><a href="/register">CTA</a></p>
</div>

(rendered as inline-blocks floated left)

The former being a conscious decision to add semantic meaning to the section, while the later is just used for styling purposes

@trieloff
Copy link
Contributor

trieloff commented May 7, 2019

It seems that you are arguing using a couple of assumptions that don't reflect how Helix works or is designed, so I'm trying my best to clarify.

content-aware presentation

Whether you represent your section as gallery, or hero, or list, or multiple <p/>, does not change the content it has…

In Helix, the content is driving the presentation. The content, not metadata, not templates or other rules. This means that just by having lots of images in a section, Helix can identify that this section is a gallery. Just by having a singular image, a headline and a short text with a link at the beginning of the document, Helix can identify that this section is a hero, and so on. Helix is able to look into the content of a section to determine what kind of section it is.

Right now, we do this by adding class attribute values to the section HTML element we are generating, but we can absolutely imagine being smarter about this and also turning certain sections into aside, or main elements, assuming we can reliably detect the main part of the content.

However, in order to get there, we need to give the author the ability to explicitly structure their document, which we do with --- in Markdown. Headlines (as suggested in #86 implicitly structure the document, but would be surprising to most authors, and defeat the purpose of content-detection, because sections would become too small)

content targeted for specific audiences or use cases

Let's take blind vs. color blind vs. person with no disability vs. mobile.

Not a good example, because this is the same content, just presented differently. Additionally, there are plenty of technologies available to support this already (alt, aria-, media queries).

A better example would be this page: https://www.adobe.com/products/photoshop.html#!

What would it look like if we could differentiate between:

  • people who visit the page for the first time
  • people who have started to subscribe, but didn't finish
  • people who just started their subscription
  • people who are active users of Photoshop

As an author, you'd want to treat this as a consistent whole, but have special sections for different users. The first-timer would get an introduction what Photoshop is, the hesitant buyer would get arguments to close the purchase, the new subscriber would get getting started guides and tutorials, the active user would get links to related products that work great with Photoshop.

Each of these sections has dramatically different content, each of these sections works on it's own and is a real <section> for HTML (at least much more a section than a bland div).

variations of content for the purpose of A/B testing

Again, semantics will be the same for each version, just the content would be different. The fact that you render <img/>+<p/>+<button/> centered in A while you may be using <p/>+<img/>+<button/>+<p/> aligned right in B still is just a design decision that does not change the semantic block or the document outline.

What you are testing in your example is not content variations, but style variations. A good use case for content-based variations is the first section ("Get Photoshop as part of Adobe Creative Cloud for just US$20.99/mo"). Variations to test could include:

  • get Photoshop as a stand-alone app ($21)
  • get it with the entire Creative Cloud ($53)
  • get it in the Photography plan ($10)
  • get it with the whole kitchen sink and Adobe Stock ($83)
  • emphasize the free trial
  • focus on the yearly price, etc.

Again, you have a logical section of the document that should get rendered as a section.


You are right, this is absolutely an opinionated choice on behalf of the framework. If your Markdown sections don't translate well to HTML sections, you might be misusing them.

@ramboz
Copy link
Contributor

ramboz commented May 7, 2019

It seems that you are arguing using a couple of assumptions that don't reflect how Helix works or is designed, so I'm trying my best to clarify.

Definitely! Still new to the project and trying to catch up on all the concept and assumptions. So I really do appreciate you taking the time to clarify these concepts here.

I understand that structuring purely based on headings may not be a good call:

  • it's an opinionated decision from the framework
  • as @koraa pointed out the implicit sectioning is not officially part of the HTML5 spec
  • it's confusing for the author as the markup is not structured as the markdown

A better example would be this page: https://www.adobe.com/products/photoshop.html#!

Agreed, great example. As I pointed out, I'm missing the initial use cases that we want to cover. That clarifies it, and I can clearly see a completely different structure needed for each. I can also see the need to use 1 or more <sections/> for each.

I completely agree on the need to section the content, and with the examples you give, I have now a better understanding of the use cases we want to cover. Assuming the author does not need to know about the differences between <main/>, <article/>, <section/>, <aside/>, etc. and assuming the framework intelligence is supposed to pick the most appropriate one for each use case, I'd still probably want to make this somehow configurable/overridable and fallback to a generic non-semantic <div/>. The reason being that I may want to enforce a specific tag:

  • for accessibility reasons
  • for SEO reasons
  • for styling reasons
  • or just because the framework is not "smart-enough" yet to guess the proper tag

Just trying to point out that using the actual <section/> tag by default instead of a <div class="section"/> does actually have side-effects that most authors do not control/understand.

@davidnuescheler
Copy link
Contributor Author

I think @ramboz is correct on the potential abuse" of sections for css and js targeting.
If I think of a traditional homepage though with different sections, the actual thematic break is a fairly good approximation.

I can totally get behind using div.section instead especially as my first prototype did exactly that.

@ramboz ramboz self-assigned this Jun 14, 2019
ramboz added a commit that referenced this issue Jun 14, 2019
ramboz added a commit that referenced this issue Jun 15, 2019
Include proper sections in the HTML pipeline instead of plain <hr/> sectioning

BREAKING CHANGE: Remove `context.content.sections` and move the sections directly under
`context.content.mdast`; move the section types to the mdast `<node>.data.types`; move the section
yaml meta info to `<node>.data.meta`

fix #279
ramboz added a commit that referenced this issue Jun 17, 2019
Include proper sections in the HTML pipeline instead of plain <hr/> sectioning

fix #279
ramboz added a commit that referenced this issue Jun 21, 2019
refactor existing tests and adjust code to properly support the new sections in the mdast

fix #279
ramboz added a commit that referenced this issue Jun 22, 2019
ramboz added a commit that referenced this issue Jun 22, 2019
Include proper sections in the HTML pipeline instead of plain <hr/> sectioning

BREAKING CHANGE: Remove `context.content.sections` and move the sections directly under
`context.content.mdast`; move the section types to the mdast `<node>.data.types`; move the section
yaml meta info to `<node>.data.meta`

fix #279
ramboz added a commit that referenced this issue Jun 22, 2019
Include proper sections in the HTML pipeline instead of plain <hr/> sectioning

fix #279
ramboz added a commit that referenced this issue Jun 22, 2019
refactor existing tests and adjust code to properly support the new sections in the mdast

fix #279
ramboz added a commit that referenced this issue Jun 22, 2019
ramboz added a commit that referenced this issue Jun 23, 2019
ramboz added a commit that referenced this issue Jun 28, 2019
BREAKING CHANGE: section types are moved to the sectio's meta object

fix #279
ramboz added a commit that referenced this issue Jul 5, 2019
Include proper sections in the HTML pipeline instead of plain `<hr>` sectioning

BREAKING CHANGE:
- Remove `context.content.sections` and move the sections directly under
`context.content.mdast`
- Adjust json schemas accordingly
- Move the section yaml meta info to `<node>.meta`
- Move the section types to the mdast `<node>.meta.types`

Is dependent on adobe/helix-cli#1037

fix #279
adobe-bot pushed a commit that referenced this issue Jul 5, 2019
# [4.0.0](v3.7.4...v4.0.0) (2019-07-05)

### Features

* **html pipe:** Default pipeline should include <section/>s ([#379](#379)) ([27ad875](27ad875)), closes [adobe/helix-cli#1037](adobe/helix-cli#1037) [#279](#279)

### BREAKING CHANGES

* **html pipe:** - Remove `context.content.sections` and move the sections directly under
`context.content.mdast`
- Adjust json schemas accordingly
- Move the section yaml meta info to `<node>.meta`
- Move the section types to the mdast `<node>.meta.types`
@adobe-bot
Copy link

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

ramboz added a commit to adobe/helix-cli that referenced this issue Jul 5, 2019
ramboz added a commit to adobe/helix-cli that referenced this issue Jul 5, 2019
…he meta JSON attributes

Fix integration tests for new sections model, by adding the newly exposed types array on the meta object.

fix adobe/helix-pipeline#279
@trieloff trieloff mentioned this issue Nov 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.