VDOM Util #53

trieloff · 2018-09-19T12:21:20Z

codecov · 2018-09-19T12:21:31Z

Codecov Report

Merging #53 into master will increase coverage by 0.1%.
The diff coverage is 97.64%.

@@            Coverage Diff            @@
##           master      #53     +/-   ##
=========================================
+ Coverage    97.1%   97.21%   +0.1%     
=========================================
  Files          18       20      +2     
  Lines         346      431     +85     
  Branches       58       68     +10     
=========================================
+ Hits          336      419     +83     
- Misses         10       12      +2

Impacted Files	Coverage Δ
src/utils/index.js	`100% <100%> (ø)`	⬆️
src/html/make-html.js	`100% <100%> (ø)`	⬆️
src/utils/image-handler.js	`100% <100%> (ø)`
src/utils/mdast-to-vdom.js	`100% <100%> (ø)`
src/html/output-debug.js	`88.88% <33.33%> (-11.12%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4494127...82b34fe. Read the comment docs.

…HTAST

trieloff · 2018-09-24T09:57:09Z

@kptdobe please have a look.

If we merge this, we can remove content.htast and content.html (and probably content.children) as they are redundant with content.document, content.document.innerHTML and content.document.children.

kptdobe · 2018-09-24T10:07:08Z

ok, I'll review this afternoon.

tripodsan

in VDOMTRansformer:

do you really want to export all static methods?
it would be good to add js doc to all methods and declare the ones considered private with @private.

tripodsan · 2018-09-24T13:22:30Z

src/utils/mdast-to-vdom.js

+      });
+  }
+
+  static toHTAST(htmlstr, cb, node) {


tripodsan · 2018-09-24T13:22:37Z

src/utils/mdast-to-vdom.js

+    return cb(node, 'div', {}, htast.children);
+  }
+
+  static handle(cb, node, parent, that) {


tripodsan · 2018-09-24T13:22:58Z

src/utils/mdast-to-vdom.js

+    return result;
+  }
+
+  static default(node) {


tripodsan · 2018-09-24T13:23:13Z

src/utils/mdast-to-vdom.js

+    return handlers[node.type];
+  }
+
+  match(matcher, processor) {


tripodsan · 2018-09-24T13:23:20Z

src/utils/mdast-to-vdom.js

+    this._matchers.push([matchfn, processor]);
+  }
+
+  matches(node) {


tripodsan · 2018-09-24T13:23:31Z

src/utils/mdast-to-vdom.js

+    return processors[0];
+  }
+
+  static matchfn(ast, pattern) {


tripodsan · 2018-09-24T13:23:45Z

src/utils/mdast-to-vdom.js

+    };
+  }
+
+  process() {


tripodsan · 2018-09-24T13:23:55Z

src/utils/mdast-to-vdom.js

+    return tohyper(h, tohast(this._root, { handlers: this._handlers }));
+  }
+
+  getDocument() {


kptdobe · 2018-09-24T14:03:29Z

I tried the code with the initial project (hlx demo test) and the document object contains only an "blank" location object (no children, no innerHTML...)
I tested by using the console output of the payload requesting: http://localhost:3000/index.html?debug=true

kptdobe · 2018-09-24T15:45:54Z

It is the "JSONification" of the document object which does not work. A breakpoint in the pre.js shows me the content.document object full of stuff!

Implemented all suggestions.

- addressing @kptadobe's comment: #53 (comment)

kptdobe · 2018-09-25T09:25:10Z

I started to look into this PR and I really do not understand where we go with this:

we now have a huge content.document object which is a representation of the browser DOM, including all events (onmousemove...) which is completely useless. Just for the sake of having some helper methods like 'querySelector' or 'querySelectorAll' there are millions of useless stuff around.
I still do not understand who is going to use things like content.document.getElementsByTagName('h1') and to do what... My assumption is that a web developer will want to work with HTL and a developer preparing the data or convertir the sections into something project specific might want to go more for a simplified version of the mdast, basically a json representation.

This looks sooo complicated for the needs we have at the moment.

If you could illustrate how this would be used, that would be great.

trieloff · 2018-09-25T12:09:53Z

The basic motivation for this is outlined in @simonwex's issue adobe/project-helix#305:

Mark

Mark is using Helix to power the adobe.io website by outputting HTML
Mark needs to restructure the content to efficiently produce attractive layouts without the adobe.io content authors having to have special formatting knowledge

When styling the content, he uses CSS selectors to identify content

When manipulating the content structure, he works with the DOM API

So the idea is to give Mark an API for manipulating HTML that he is familiar with, instead of forcing him to learn HTAST (as great as it might be with all the unist-utils) or any new content structure (that we haven't defined in more than a couple of months, see #24)

we now have a huge content.document object which is a representation of the browser DOM, including all events (onmousemove…) which is completely useless.

It might be useless, but I'd argue there is little harm done and it can increase compatibility when the developer is introducing their favorite library that happens to access onmousemove without any guards.

I still do not understand who is going to use things like content.document.getElementsByTagName('h1') and to do what... My assumption is that a web developer will want to work with HTL and a developer preparing the data or convertir the sections into something project specific might want to go more for a simplified version of the mdast, basically a json representation.

Below are a couple of examples of how I could see this play out. They all have in common that the developer does not want to touch anything that looks like Markdown, but is very familiar with the DOM.

Super Simple

Mark is fine with the HTML content and just wants it printed as is:

<body>
${content.document.innerHTML}
</body>

Easy.

Wrapping top-level elements

This replaces the it.children iterator.

<div data-sly-list="${content.document.childNodes}">${item.innerHTML}</div>

Adjusting Headlines

This takes a little work in pre.js:

// remove the first heading
const firstheading = content.document.querySelector('h1');
firstheading.parentNode.removeChild(firstheading);

// downgrade all remaining h1 to h2
content.document.getElementsByTagName('h1').map(heading => {
  heading.outerHTML = '<h2>' + heading.innerHTML + '</h2>';
});

<body>
<h1 class="myheading">${content.meta.title}</h1>
${content.document.innerHTML}
</body>

In all examples, you could even prototype the code on the client, in the web developer console before moving it server-side.

simonwex · 2018-09-25T19:01:32Z

This is much tidier than I thought it would be. You nailed the use-case. Nice work!

I'll try to use it for real at some point this week, but from my static inspection, it looks like a great candidate to merge.

trieloff · 2018-09-26T07:36:58Z

@kptdobe @simonwex I'm putting this back in WIP, as I noticed that the PR would break the image responsifyer, which is working on HTAST. I will need to re-implement that on top of the new DOM.

trieloff · 2018-09-26T15:57:17Z

Some more documentation to describe how custom sizes for responsive images can be created.

trieloff added 2 commits September 19, 2018 11:01

beginning to build a customizable mdast to VDOM processor

a5254cc

Actually return VDOM nodes instead of HTAST nodes

9f29f7c

trieloff added 2 commits September 19, 2018 12:45

Testing the simple case

98412e2

deal gracefully with handlers that return HTML as strings instead of …

f1f2466

…HTAST

trieloff mentioned this pull request Sep 21, 2018

Section Pattern Matching Language inspired by RegExp #54

Merged

5 tasks

trieloff added 7 commits September 24, 2018 07:44

Merge branch 'master' into util-vdomino

d07fa03

Broader testing

13ad6e4

Merge branch 'master' into util-vdomino

e7c936c

test and example for returning DOM nodes

59d540d

get 100% branch coverage

7c11b06

code style

bbda4ae

documentation in progress

65b51ea

trieloff changed the title ~~WIP: VDOM Util~~ VDOM Util Sep 24, 2018

More readme

6f3ce18

trieloff added 2 commits September 24, 2018 10:25

completed README, more examples

d1f4e90

lint

bed7d62

tripodsan previously requested changes Sep 24, 2018

View reviewed changes

trieloff mentioned this pull request Sep 25, 2018

[Bootstrap Grayscale demo case] Validate new concepts kptdobe/helix-bootstrap-grayscale#1

Open

JSDoc

f1acc77

trieloff added 2 commits September 25, 2018 08:57

Merge branch 'master' into util-vdomino

b169541

Enable better debugging of content.document

c86bc42

- addressing @kptadobe's comment: #53 (comment)

tripodsan approved these changes Sep 25, 2018

View reviewed changes

fixed paren in examples

8135055

trieloff changed the title ~~VDOM Util~~ WIP: VDOM Util Sep 26, 2018

trieloff added 3 commits September 26, 2018 13:30

added custom image handler for responsive images

be6fdd5

fix initialization of VDOM transformer

1c5d272

fix lint

4e0cf37

trieloff changed the title ~~WIP: VDOM Util~~ VDOM Util Sep 26, 2018

added documentation for responsive images

a95e6fd

trieloff mentioned this pull request Sep 27, 2018

Embed External Content #56

Closed

Merge remote-tracking branch 'origin/master' into util-vdomino

82b34fe

trieloff merged commit 3b1dcab into master Oct 1, 2018

tripodsan deleted the util-vdomino branch October 2, 2018 00:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VDOM Util #53

VDOM Util #53

trieloff commented Sep 19, 2018

codecov bot commented Sep 19, 2018 •

edited

Loading

trieloff commented Sep 24, 2018

kptdobe commented Sep 24, 2018

tripodsan left a comment

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

tripodsan Sep 24, 2018

kptdobe commented Sep 24, 2018 •

edited

Loading

kptdobe commented Sep 24, 2018

kptdobe commented Sep 25, 2018 •

edited

Loading

trieloff commented Sep 25, 2018

Mark

simonwex commented Sep 25, 2018

trieloff commented Sep 26, 2018

trieloff commented Sep 26, 2018

VDOM Util #53

VDOM Util #53

Conversation

trieloff commented Sep 19, 2018

codecov bot commented Sep 19, 2018 • edited Loading

Codecov Report

trieloff commented Sep 24, 2018

kptdobe commented Sep 24, 2018

tripodsan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kptdobe commented Sep 24, 2018 • edited Loading

kptdobe commented Sep 24, 2018

kptdobe commented Sep 25, 2018 • edited Loading

trieloff commented Sep 25, 2018

Mark

Super Simple

Wrapping top-level elements

Adjusting Headlines

simonwex commented Sep 25, 2018

trieloff commented Sep 26, 2018

trieloff commented Sep 26, 2018

codecov bot commented Sep 19, 2018 •

edited

Loading

kptdobe commented Sep 24, 2018 •

edited

Loading

kptdobe commented Sep 25, 2018 •

edited

Loading