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

Loose vs tight lists - demo #75

Closed
wants to merge 2 commits into from
Closed

Loose vs tight lists - demo #75

wants to merge 2 commits into from

Conversation

TRIAEIOU
Copy link
Contributor

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

As per earlier discussion
TODO

@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Aug 24, 2022
@github-actions

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Sep 10, 2022

Thanks, I’ve thought about this for a bit, and I think you’re right that this is a good start, but I wanted to expand it to two things: a) 2 or more “flow” elements are also spread out, other “flow” elements should be searched if they themselves contain a <p> or a).

Here’s the diff I came up with:

+++ b/lib/handlers/li.js
@@ -6,6 +6,7 @@
  */
 
 import {convertElement} from 'hast-util-is-element'
+import {phrasing} from 'hast-util-phrasing'
 import {wrapChildren} from '../util/wrap-children.js'
 
 const p = convertElement('p')
@@ -45,7 +46,52 @@ export function li(h, node) {
     }
   }
 
-  const content = wrapChildren(h, clone || node)
+  if (!clone) clone = node
 
-  return h(node, 'listItem', {spread: content.length > 1, checked}, content)
+  const spread = spreadout(clone)
+  const content = wrapChildren(h, clone)
+
+  return h(node, 'listItem', {spread, checked}, content)
+}
+
+/**
+ * Check if an element should spread out.
+ * The reason to spread out a markdown list item is primarily whether writing
+ * the equivalent in markdown, would yield a spread out item.
+ *
+ * A spread out item results in `<p>` and `</p>` tags.
+ * Otherwise, the phrasing would be output directly.
+ * We can check for that: if there’s a `<p>` element, spread it out.
+ *
+ * But what if there are no paragraphs?
+ * In that case, we can also assume that if two “block” things were written in
+ * an item, that it is spread out, because blocks are typically joined by blank
+ * lines, which also means a spread item.
+ *
+ * Lastly, because in HTML things can be wrapped in a `<div>` or similar, we
+ * delve into non-phrasing elements here to figure out if they themselves
+ * contain paragraphs or 2 or more flow non-phrasing elements.
+ *
+ * @param {Element} node
+ * @returns {boolean}
+ */
+function spreadout(node) {
+  let index = -1
+  let seenFlow = false
+
+  while (++index < node.children.length) {
+    const child = node.children[index]
+
+    if (child.type === 'element') {
+      if (phrasing(child)) continue
+
+      if (child.tagName === 'p' || seenFlow || spreadout(child)) {
+        return true
+      }
+
+      seenFlow = true
+    }
+  }
+
+  return false
 }

It seems to work. Also has some errors of course, which I think are improvements.

@wooorm
Copy link
Member

wooorm commented Sep 19, 2022

@TRIAEIOU Friendly ping! :)

wooorm added a commit that referenced this pull request Oct 12, 2022
Closes GH-75.

Co-authored-by: TRIAEIOU <94647023+TRIAEIOU@users.noreply.github.com>
@wooorm
Copy link
Member

wooorm commented Oct 12, 2022

Made this into a PR: #76.

@TRIAEIOU
Copy link
Contributor Author

@TRIAEIOU Friendly ping! :)

Hi, sorry, I got caught up in other stuff. That looks great!

Thanks!

@wooorm wooorm closed this in 9706341 Jan 12, 2023
@wooorm wooorm added the 💪 phase/solved Post is done label Jan 12, 2023
@github-actions

This comment has been minimized.

@github-actions github-actions bot removed the 👋 phase/new Post is being triaged automatically label Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants