From f215eeedac66e08cbee9f79c0573a566f4480f40 Mon Sep 17 00:00:00 2001 From: Daryl Tan <3646725+openorclose@users.noreply.github.com> Date: Tue, 3 Mar 2020 21:48:07 +0800 Subject: [PATCH] Migrate to bootstrap-vue popovers (#1033) --- asset/css/markbind.css | 29 ++++ asset/js/setup.js | 33 ++++ docs/userGuide/syntax/extra/triggers.mbdf | 2 +- docs/userGuide/syntax/modals.mbdf | 4 +- docs/userGuide/syntax/popovers.mbdf | 6 +- docs/userGuide/syntax/tooltips.mbdf | 5 +- .../markbind/src/parsers/componentParser.js | 152 ++++++++++++++++-- .../test_site/expected/bugs/index.html | 35 ++-- test/functional/test_site/expected/index.html | 26 ++- .../expected/markbind/css/markbind.css | 29 ++++ .../test_site/expected/markbind/js/setup.js | 33 ++++ .../test_site/expected/siteData.json | 1 + .../expected/testTooltipSpacing.html | 6 +- .../expected/index.html | 16 +- .../expected/markbind/css/markbind.css | 29 ++++ .../expected/markbind/js/setup.js | 33 ++++ .../expected/markbind/css/markbind.css | 29 ++++ .../expected/markbind/js/setup.js | 33 ++++ .../expected/markbind/css/markbind.css | 29 ++++ .../expected/markbind/js/setup.js | 33 ++++ .../test_default/expected/index.html | 12 +- .../expected/markbind/css/markbind.css | 29 ++++ .../expected/markbind/js/setup.js | 33 ++++ .../expected/markbind/css/markbind.css | 29 ++++ .../expected/markbind/js/setup.js | 33 ++++ test/unit/parsers/componentParser.test.js | 2 +- test/unit/utils/componentParserData.js | 52 +++--- 27 files changed, 640 insertions(+), 113 deletions(-) diff --git a/asset/css/markbind.css b/asset/css/markbind.css index b38ecfa195..d38eb2b900 100644 --- a/asset/css/markbind.css +++ b/asset/css/markbind.css @@ -340,3 +340,32 @@ li.footnote-item:target { top: 0; width: 3em; } + +/* hide popover, modal, tooltip content */ +[data-mb-html-for] { + display: none; +} + +/* styles for triggers */ +.trigger { + text-decoration: underline dotted; +} + +.modal.mb-zoom { + -webkit-transform: scale(0.1); + -moz-transform: scale(0.1); + -ms-transform: scale(0.1); + transform: scale(0.1); + opacity: 0; + -webkit-transition: all 0.3s; + -moz-transition: all 0.3s; + transition: all 0.3s; +} + +.modal.mb-zoom.show { + -webkit-transform: scale(1); + -moz-transform: scale(1); + -ms-transform: scale(1); + transform: scale(1); + opacity: 1; +} diff --git a/asset/js/setup.js b/asset/js/setup.js index 114e5a70e8..c6b54ef24f 100644 --- a/asset/js/setup.js +++ b/asset/js/setup.js @@ -171,6 +171,39 @@ function setupWithSearch() { setupSiteNav(); } +function makeInnerGetterFor(attribute) { + return (element) => { + const innerElement = element.querySelector(`[data-mb-html-for="${attribute}"]`); + return innerElement === null ? '' : innerElement.innerHTML; + }; +} + +function makeHtmlGetterFor(componentType, attribute) { + return (element) => { + const contentWrapper = document.getElementById(element.attributes.for.value); + return contentWrapper.dataset.mbComponentType === componentType + ? makeInnerGetterFor(attribute)(contentWrapper) : ''; + }; +} + +/* eslint-disable no-unused-vars */ +/* + These getters are used by triggers to get their popover/tooltip content. + We need to create a completely new popover/tooltip for each trigger due to bootstrap-vue's implementation, + so this is how we retrieve our contents. +*/ +const popoverContentGetter = makeHtmlGetterFor('popover', 'content'); +const popoverHeaderGetter = makeHtmlGetterFor('popover', 'header'); +const popoverInnerContentGetter = makeInnerGetterFor('content'); +const popoverInnerHeaderGetter = makeInnerGetterFor('header'); + +const popoverGenerator = { title: popoverHeaderGetter, content: popoverContentGetter }; +const popoverInnerGenerator = { title: popoverInnerHeaderGetter, content: popoverInnerContentGetter }; + +const tooltipContentGetter = makeHtmlGetterFor('tooltip', '_content'); +const tooltipInnerContentGetter = makeInnerGetterFor('_content'); +/* eslint-enable no-unused-vars */ + if (enableSearch) { setupWithSearch(); } else { diff --git a/docs/userGuide/syntax/extra/triggers.mbdf b/docs/userGuide/syntax/extra/triggers.mbdf index 56d70d2b91..ff6d7d0edd 100644 --- a/docs/userGuide/syntax/extra/triggers.mbdf +++ b/docs/userGuide/syntax/extra/triggers.mbdf @@ -14,5 +14,5 @@ Additionally, multiple Triggers could share the same overlay by providing them w Name | Type | Default | Description ---- | ---- | ------- | ------ -trigger | `String` | `hover` | How the overlay view is triggered.
Supports: `click`, `focus`, `hover`, `contextmenu`. +trigger | `String` | `hover` | How the overlay view is triggered.
Supports: `click`, `focus`, `hover`. for | `String` | `null` | The id for the overlay view to be shown. \ No newline at end of file diff --git a/docs/userGuide/syntax/modals.mbdf b/docs/userGuide/syntax/modals.mbdf index cdfbe0abea..1f46638ffd 100644 --- a/docs/userGuide/syntax/modals.mbdf +++ b/docs/userGuide/syntax/modals.mbdf @@ -54,9 +54,7 @@ Name | type | Default | Description header
title
(deprecated)
| `String` | `''` | Header of the Modal component. Supports inline markdown text. ok-text | `String` | `''` | Text for the OK button. effect | `String` | `zoom` | Supports: `zoom`, `fade`. -id | `String` | | Used by [Trigger](#trigger) to activate the Modal by id. -width | `Number`, `String`, or `null` | `null` | Passing a `Number` will be translated to pixels.
`String` can be passed [CSS units](https://www.w3schools.com/cssref/css_units.asp), ( e.g. '50in' or '30vw' ).
`null` will default to Bootstrap's responsive sizing. -large | `Boolean` | `false` | Creates a [large Modal](https://getbootstrap.com/docs/4.0/components/modal/#optional-sizes). +id | `String` | | Used by [Trigger](#trigger) to activate the Modal by id.large | `Boolean` | `false` | Creates a [large Modal](https://getbootstrap.com/docs/4.0/components/modal/#optional-sizes). small | `Boolean` | `false` | Creates a [small Modal](https://getbootstrap.com/docs/4.0/components/modal/#optional-sizes). center | `Boolean` | `false` | Vertically centers the modal (in addition to the horizontal centering by default). backdrop | `Boolean` | `true` | Enables closing the Modal by clicking on the backdrop. diff --git a/docs/userGuide/syntax/popovers.mbdf b/docs/userGuide/syntax/popovers.mbdf index 279fab7fc4..143e8949f2 100644 --- a/docs/userGuide/syntax/popovers.mbdf +++ b/docs/userGuide/syntax/popovers.mbdf @@ -93,9 +93,6 @@ - - -

Markdown

@@ -146,8 +143,7 @@ This is the same trigger as last one. Name | Type | Default | Description ---- | ---- | ------- | ------ -trigger | `String` | `hover` | How the Popover is triggered.
Supports: `click`, `focus`, `hover`, `contextmenu`. -effect | `String` | `fade` | Transition effect for Popover.
Supports: `scale`, `fade`. +trigger | `String` | `hover` | How the Popover is triggered.
Supports: `click`, `focus`, `hover`. header


title
(deprecated)
| `String` | `''` | Popover header, supports inline markdown text. content | `String` | `''` | Popover content, supports inline markdown text. placement | `String` | `top` | How to position the Popover.
Supports: `top`, `left`, `right`, `bottom`. diff --git a/docs/userGuide/syntax/tooltips.mbdf b/docs/userGuide/syntax/tooltips.mbdf index b8c9949ac5..ea05efaa24 100644 --- a/docs/userGuide/syntax/tooltips.mbdf +++ b/docs/userGuide/syntax/tooltips.mbdf @@ -61,9 +61,6 @@ Trigger - - -

@@ -111,7 +108,7 @@ This is the same trigger as last one. Name | Type | Default | Description ---- | ---- | ------- | ------ -trigger | `String` | `hover` | How the tooltip is triggered.
Supports: `click`, `focus`, `hover`, `contextmenu`. +trigger | `String` | `hover` | How the tooltip is triggered.
Supports: `click`, `focus`, `hover`. content | `String` | `''` | Text content of the tooltip. placement | `String` | `top` | How to position the tooltip.
Supports: `top`, `left`, `right`, `bottom`. diff --git a/src/lib/markbind/src/parsers/componentParser.js b/src/lib/markbind/src/parsers/componentParser.js index 982fc2b596..3513c87fe6 100644 --- a/src/lib/markbind/src/parsers/componentParser.js +++ b/src/lib/markbind/src/parsers/componentParser.js @@ -42,6 +42,27 @@ function _parseAttributeWithoutOverride(element, attribute, isInline, slotName = delete el.attribs[attribute]; } +/** + * Takes an element, looks for direct elements with slots and transforms to avoid Vue parsing. + * This is so that we can use bootstrap-vue popovers, tooltips, and modals. + * @param element Element to transform + */ +function _transformSlottedComponents(element) { + element.children.forEach((child) => { + const c = child; + const slot = c.attribs && c.attribs.slot; + if (slot) { + // Turns
... into
... + c.attribs['data-mb-html-for'] = slot; + delete c.attribs.slot; + } + // similarly, need to transform templates to avoid Vue parsing + if (c.name === 'template') { + c.name = 'span'; + } + }); +} + /* * Panels */ @@ -119,30 +140,80 @@ function _assignPanelId(element) { } } +/* + * Triggers + * + * At "compile time", we can't tell whether a trigger references a modal, popover, or toolip, + * since that element might not have been processed yet. + * + * So, we make every trigger try all 3. It will attempt to open a tooltip, popover, and modal. + * + * For tooltips and popovers, we call the relevant content getters inside asset/js/setup.js. + * They will check to see if the element id exists, and whether it is a popover/tooltip, + * and then return the content as needed. + * + * For modals, we make it attempt to show the modal if it exists. + */ + +function _parseTrigger(element) { + const el = element; + el.name = 'span'; + const trigger = el.attribs.trigger || 'hover'; + const placement = el.attribs.placement || 'top'; + el.attribs[`v-b-popover.${trigger}.${placement}.html`] + = 'popoverGenerator'; + el.attribs[`v-b-tooltip.${trigger}.${placement}.html`] + = 'tooltipContentGetter'; + const convertedTrigger = trigger === 'hover' ? 'mouseover' : trigger; + el.attribs[`v-on:${convertedTrigger}`] = `$refs['${el.attribs.for}'].show()`; + el.attribs.class = el.attribs.class ? `${el.attribs.class} trigger` : 'trigger'; +} /* * Popovers + * + * We hide the content and header via _transformSlottedComponents, for retrieval by triggers. + * + * Then, we add in a trigger for this popover. */ -function _parsePopoverAttributes(element) { - _parseAttributeWithoutOverride(element, 'content', true); - _parseAttributeWithoutOverride(element, 'header', true); +function _parsePopover(element) { + const el = element; + _parseAttributeWithoutOverride(el, 'content', true); + _parseAttributeWithoutOverride(el, 'header', true); // TODO deprecate title attribute for popovers - _parseAttributeWithoutOverride(element, 'title', true, 'header'); + _parseAttributeWithoutOverride(el, 'title', true, 'header'); + + el.name = 'span'; + const trigger = el.attribs.trigger || 'hover'; + const placement = el.attribs.placement || 'top'; + el.attribs['data-mb-component-type'] = 'popover'; + el.attribs[`v-b-popover.${trigger}.${placement}.html`] + = 'popoverInnerGenerator'; + el.attribs.class = el.attribs.class ? `${el.attribs.class} trigger` : 'trigger'; + _transformSlottedComponents(el); } /* * Tooltips + * + * Similar to popovers. */ -function _parseTooltipAttributes(element) { - _parseAttributeWithoutOverride(element, 'content', true, '_content'); +function _parseTooltip(element) { + const el = element; + _parseAttributeWithoutOverride(el, 'content', true, '_content'); + + el.name = 'span'; + const trigger = el.attribs.trigger || 'hover'; + const placement = el.attribs.placement || 'top'; + el.attribs['data-mb-component-type'] = 'tooltip'; + el.attribs[`v-b-tooltip.${trigger}.${placement}.html`] + = 'tooltipInnerContentGetter'; + el.attribs.class = el.attribs.class ? `${el.attribs.class} trigger` : 'trigger'; + _transformSlottedComponents(el); } -/* - * Modals - */ - function _renameSlot(element, originalName, newName) { if (element.children) { element.children.forEach((c) => { @@ -155,14 +226,60 @@ function _renameSlot(element, originalName, newName) { } } +function _renameAttribute(element, originalAttribute, newAttribute) { + const el = element; + if (_.has(el.attribs, originalAttribute)) { + el.attribs[newAttribute] = el.attribs[originalAttribute]; + delete el.attribs[originalAttribute]; + } +} + +/* + * Modals + * + * We are using bootstrap-vue modals, and some of their attributes/slots differ from ours. + * So, we will transform from markbind modal syntax into bootstrap-vue modal syntax. + */ + function _parseModalAttributes(element) { - _parseAttributeWithoutOverride(element, 'header', true, '_header'); + const el = element; + _parseAttributeWithoutOverride(el, 'header', true, 'modal-title'); // TODO deprecate title attribute for modals - _parseAttributeWithoutOverride(element, 'title', true, '_header'); + _parseAttributeWithoutOverride(el, 'title', true, 'modal-title'); // TODO deprecate modal-header, modal-footer attributes for modals - _renameSlot(element, 'modal-header', 'header'); - _renameSlot(element, 'modal-footer', 'footer'); + _renameSlot(el, 'header', 'modal-header'); + _renameSlot(el, 'footer', 'modal-footer'); + + el.name = 'b-modal'; + + _renameAttribute(el, 'ok-text', 'ok-title'); + _renameAttribute(el, 'center', 'centered'); + + el.attribs['ok-only'] = ''; // only show OK button + + if (el.attribs.backdrop === 'false') { + el.attribs['no-close-on-backdrop'] = ''; + } + delete el.attribs.backdrop; + + let size = ''; + if (_.has(el.attribs, 'large')) { + size = 'lg'; + delete el.attribs.large; + } else if (_.has(el.attribs, 'small')) { + size = 'sm'; + delete el.attribs.small; + } + el.attribs.size = size; + + // default for markbind is zoom, default for bootstrap-vue is fade + const effect = el.attribs.effect === 'fade' ? '' : 'mb-zoom'; + el.attribs['modal-class'] = effect; + + if (_.has(el.attribs, 'id')) { + el.attribs.ref = el.attribs.id; + } } /* @@ -224,11 +341,14 @@ function parseComponents(element, errorHandler) { case 'panel': _parsePanelAttributes(element); break; + case 'trigger': + _parseTrigger(element); + break; case 'popover': - _parsePopoverAttributes(element); + _parsePopover(element); break; case 'tooltip': - _parseTooltipAttributes(element); + _parseTooltip(element); break; case 'modal': _parseModalAttributes(element); diff --git a/test/functional/test_site/expected/bugs/index.html b/test/functional/test_site/expected/bugs/index.html index b64663b08a..58c921ab63 100644 --- a/test/functional/test_site/expected/bugs/index.html +++ b/test/functional/test_site/expected/bugs/index.html @@ -38,44 +38,35 @@

popover initiated by trigger: honor trigger attribute

Issue #49

Repro:

-

- Establishing Requirements -

- -
-
-

Requirements gathering, requirements elicitation, requirements analysis, requirements capture are some of the terms commonly and interchangeably used to represent the activity of understanding what a software product should - do. -

-
-
-
+

Establishing Requirements

+ +
+
+

Requirements gathering, requirements elicitation, requirements analysis, +requirements capture are some of the terms commonly and interchangeably used to represent the activity +of understanding what a software product should do.

Support multiple inclusions of a modal

Issue #107

Repro:

-

This is to reproduce - multiple inclusions of a modal bug -

- +

This is to reproduce multiple inclusions of a modal bug

+

Requirements gathering, requirements elicitation, requirements analysis, requirements capture are some of the terms commonly and interchangeably used to represent the activity of understanding what a software product should do.

-
+
-

This is to reproduce - multiple inclusions of a modal bug -

- +

This is to reproduce multiple inclusions of a modal bug

+

Requirements gathering, requirements elicitation, requirements analysis, requirements capture are some of the terms commonly and interchangeably used to represent the activity of understanding what a software product should do.

-
+

Remove extra space in links

Issue #147

diff --git a/test/functional/test_site/expected/index.html b/test/functional/test_site/expected/index.html index aa9b11cc31..dcf2201aa4 100644 --- a/test/functional/test_site/expected/index.html +++ b/test/functional/test_site/expected/index.html @@ -146,16 +146,9 @@

Testing Site-Nav

Test footnotes

-

Normal footnotes: Here is a footnote reference, - [1] and another. - [2] -

-

Here is a repeated footnote to - [1] -

-

Inline footnotes: Here is an inline note. - [3] -

+

Normal footnotes: Here is a footnote reference,[1] and another.[2]

+

Here is a repeated footnote to [1]

+

Inline footnotes: Here is an inline note.[3]

Variables that reference another variable

This variable can be referenced.

@@ -285,8 +278,8 @@

BrainstormingUser surveys

Carefully designed questionnaires can be used to solicit responses and opinions from a large number of users regarding any current system or a new innovation.

Focus groups

-

Focus groups are a kind of informal interview within an interactive group setting. A - group of people +

Focus groups are a kind of informal interview within an interactive group setting. A e.g. potential users, beta testersgroup + of people are asked about their understanding of a specific issue or a process. Focus groups can bring out undiscovered conflicts and misunderstandings among stakeholder interests which can then be resolved or clarified as necessary.

Include segment

@@ -448,14 +441,12 @@

Feature list<

Modal with panel inside

-

- trigger -

- +

trigger

+

Panel content inside modal

-
+

Unexpanded panel

Panel content of unexpanded panel should not appear in search data

@@ -570,6 +561,7 @@
Inner panel header without src‎ Panel with src from another Markbind site header‎ Panel with src from another Markbind site header‎ + Panel inside modal‎ Unexpanded panel header‎ Markbind Plugin Pre-render‎ diff --git a/test/functional/test_site/expected/markbind/css/markbind.css b/test/functional/test_site/expected/markbind/css/markbind.css index b38ecfa195..d38eb2b900 100644 --- a/test/functional/test_site/expected/markbind/css/markbind.css +++ b/test/functional/test_site/expected/markbind/css/markbind.css @@ -340,3 +340,32 @@ li.footnote-item:target { top: 0; width: 3em; } + +/* hide popover, modal, tooltip content */ +[data-mb-html-for] { + display: none; +} + +/* styles for triggers */ +.trigger { + text-decoration: underline dotted; +} + +.modal.mb-zoom { + -webkit-transform: scale(0.1); + -moz-transform: scale(0.1); + -ms-transform: scale(0.1); + transform: scale(0.1); + opacity: 0; + -webkit-transition: all 0.3s; + -moz-transition: all 0.3s; + transition: all 0.3s; +} + +.modal.mb-zoom.show { + -webkit-transform: scale(1); + -moz-transform: scale(1); + -ms-transform: scale(1); + transform: scale(1); + opacity: 1; +} diff --git a/test/functional/test_site/expected/markbind/js/setup.js b/test/functional/test_site/expected/markbind/js/setup.js index 114e5a70e8..c6b54ef24f 100644 --- a/test/functional/test_site/expected/markbind/js/setup.js +++ b/test/functional/test_site/expected/markbind/js/setup.js @@ -171,6 +171,39 @@ function setupWithSearch() { setupSiteNav(); } +function makeInnerGetterFor(attribute) { + return (element) => { + const innerElement = element.querySelector(`[data-mb-html-for="${attribute}"]`); + return innerElement === null ? '' : innerElement.innerHTML; + }; +} + +function makeHtmlGetterFor(componentType, attribute) { + return (element) => { + const contentWrapper = document.getElementById(element.attributes.for.value); + return contentWrapper.dataset.mbComponentType === componentType + ? makeInnerGetterFor(attribute)(contentWrapper) : ''; + }; +} + +/* eslint-disable no-unused-vars */ +/* + These getters are used by triggers to get their popover/tooltip content. + We need to create a completely new popover/tooltip for each trigger due to bootstrap-vue's implementation, + so this is how we retrieve our contents. +*/ +const popoverContentGetter = makeHtmlGetterFor('popover', 'content'); +const popoverHeaderGetter = makeHtmlGetterFor('popover', 'header'); +const popoverInnerContentGetter = makeInnerGetterFor('content'); +const popoverInnerHeaderGetter = makeInnerGetterFor('header'); + +const popoverGenerator = { title: popoverHeaderGetter, content: popoverContentGetter }; +const popoverInnerGenerator = { title: popoverInnerHeaderGetter, content: popoverInnerContentGetter }; + +const tooltipContentGetter = makeHtmlGetterFor('tooltip', '_content'); +const tooltipInnerContentGetter = makeInnerGetterFor('_content'); +/* eslint-enable no-unused-vars */ + if (enableSearch) { setupWithSearch(); } else { diff --git a/test/functional/test_site/expected/siteData.json b/test/functional/test_site/expected/siteData.json index 8fca18165f..52040c0517 100644 --- a/test/functional/test_site/expected/siteData.json +++ b/test/functional/test_site/expected/siteData.json @@ -45,6 +45,7 @@ "outer-nested-panel-without-src": "Outer nested panel without src", "panel-with-src-from-another-markbind-site-header": "Panel with src from another Markbind site header", "panel-with-src-from-another-markbind-site-header-2": "Panel with src from another Markbind site header", + "panel-inside-modal": "Panel inside modal", "unexpanded-panel-header": "Unexpanded panel header", "keyword-should-be-tagged-to-this-heading-not-the-panel-heading": "Keyword should be tagged to this heading, not the panel heading", "panel-normal-source-content-headings": "Panel normal source content headings", diff --git a/test/functional/test_site/expected/testTooltipSpacing.html b/test/functional/test_site/expected/testTooltipSpacing.html index b32c566779..996cceedf1 100644 --- a/test/functional/test_site/expected/testTooltipSpacing.html +++ b/test/functional/test_site/expected/testTooltipSpacing.html @@ -27,10 +27,8 @@

569: Stray space after tooltip

<tooltip>tooltip</tooltip>, test<trigger>trigger</trigger>, test
-

- tooltip, test

-

- trigger, test

+

tooltip, test

+

trigger, test