PLANET-4811 Submenu WYSIWYG

[Inwerpsel]

• Since we only need to watch for changes in the editor, we don't need
  to make the code that queries the DOM update the block when the DOM
  changes. Instead we can use useSelect to get the blocks and filter the
  headers (and potentially other blocks that can output a heading).
• Decouple fetching the headers from creating a hierarchical
  representation.
• Just fetch the headings in render of SubmenuFrontend. This component
  doesn't get re-rendered anyway, and also the headings won't change
  (unless other blocks still need to be rendered, we probably should check
  that).
• Split functions into dedicated files.
• Add backtop as an effect, which simply toggles the display of it. The
  backtop markup and style will be added to the base template in
  PLANET-4811 Add backtop markup to base template planet4-master-theme#1166
• Use plain old links instead of implementing our own behavior. Mainly
  did this to remove (a lot of) complexity from the code while doing other
  changes. We should probably evaluate if we still want to do a transition,
  though this can probably still make use of anchors.

[sagarsdeshmukh]

As we don't have inner block html content to edit, we could safely disable the Edit as HTML option for submenu block.

To disable it we need to add html:false in block support options -
supports: { html: false, },

[pablocubico]

Not a blocker: This should be safe given the content of levels but I personally prefer using LoDash's deepClone because it's safer for different types and faster, but also is more expressive when reading the code for someone that is not familiar with this technique.

[Inwerpsel]

Yes LoDash deepClone is better for this. But I actually didn't touch SubmenuLevel yet, it's still a class. Maybe it can be written in a way that doesn't require the index lookup, and maybe also doesn't need a deep clone. Then we can avoid adding a dependency on LoDash here.

[Inwerpsel]

I made a function that has the same output as the JSON switcheroo has, and is a few times faster than it. It's not as "feature" complete as Lodash, in the sense that it doesn't preserve non-built in classes, but so doesn't the JSON approach we used 🤷 There are also some exotic or deprecated JavaScript peculiarities (like Boolean objects...) that don't behave the same way as in Lodash, but we don't use them and we probably never should 😄

[Inwerpsel]

Did a quick benchmark

/usr/bin/node /home/pieter/deepclone/bm.js
JSON bytes:  18
matches JSON approach
JSON-switcheroo   :  x 1,306,315 ops/sec ±1.03% (85 runs sampled)
lodash            :  x 1,400,846 ops/sec ±1.91% (87 runs sampled)
Custom check refs :  x 5,385,561 ops/sec ±3.58% (83 runs sampled)
Fastest is Custom check refs : 
JSON bytes:  2824
matches JSON approach
JSON-switcheroo   :  x 41,676 ops/sec ±0.77% (88 runs sampled)
lodash            :  x 40,323 ops/sec ±1.27% (85 runs sampled)
Custom check refs :  x 91,897 ops/sec ±1.79% (90 runs sampled)
Fastest is Custom check refs : 
JSON bytes:  7374
matches JSON approach
JSON-switcheroo   :  x 13,925 ops/sec ±1.13% (91 runs sampled)
lodash            :  x 17,017 ops/sec ±1.41% (83 runs sampled)
Custom check refs :  x 40,898 ops/sec ±3.23% (83 runs sampled)
Fastest is Custom check refs : 
JSON bytes:  122662
matches JSON approach
JSON-switcheroo   :  x 762 ops/sec ±1.33% (85 runs sampled)
lodash            :  x 887 ops/sec ±1.30% (86 runs sampled)
Custom check refs :  x 1,873 ops/sec ±1.22% (86 runs sampled)
Fastest is Custom check refs : 
JSON bytes:  642680
matches JSON approach
JSON-switcheroo   :  x 176 ops/sec ±2.45% (69 runs sampled)
lodash            :  x 295 ops/sec ±1.61% (82 runs sampled)
Custom check refs :  x 243 ops/sec ±1.09% (81 runs sampled)
Fastest is lodash            : 

Process finished with exit code 0

[Inwerpsel]

Rebasing seems to have confused GitHub, it's showing commits in the wrong order here. However locally git log does have the right order 🤔

[Inwerpsel]

Rebasing seems to have confused GitHub, it's showing commits in the wrong order here. However locally git log does have the right order

The last rebase has un-confused GitHub 🎉

[Inwerpsel]

I initially used

return Object.assign({}, ...Object.keys(value).map(k => ({ [k]: deepClone(value[k]) })));

here, but it's actually 5 times slower than the above.

[lithrel]

Good to me 👍

Minor thing I saw while looking at the Privacy & Cookies page, headings can be transformed to bold/italic (:shrug:) and the tags appear in the editor (doesn't seem to be a problem on the frontend so not blocking and probably not that frequent).

[Inwerpsel]

Good to me

Minor thing I saw while looking at the Privacy & Cookies page, headings can be transformed to bold/italic () and the tags appear in the editor (doesn't seem to be a problem on the frontend so not blocking and probably not that frequent).

Good catch 👌 Checking if I can improve that, maybe just stripping tags will do here .

Noticed another issue we probably want to fix first: the previews of the styles in the sidebar all look the same, because the block's output is based on the current headings of the post, but these don't seem to get used in the preview. I'll see if I can make it work with the current headings of the post. If not we'll have to rework the block so that it allows to be rendered from a list of static headings as well, so we can just include those literally in the example data of the block.

[Inwerpsel]

I noticed this issue for the Submenu block, so I made these changes here as well already.

[lithrel]

Some of the examples don't have an anchor (I think ?), so react raises an error

Warning: Each child in a list should have a unique "key" prop.

Check the render method of `SubmenuItems`. See https://fb.me/react-warning-keys for more information.
    in li (created by SubmenuItems)

Maybe if there's no anchor we can use the array index as a key

[Inwerpsel]

Good catch 👌

[Inwerpsel]

I used the title as an anchor in the example attributes