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

Implement src attribute for popovers #1780

Merged
merged 27 commits into from
Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions docs/userGuide/syntax/popovers.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@
</popover>
<br />
<br />
<h4 class="no-index">Content using src</h4>
<p>
<popover header="From a HTML file" src="{{ baseUrl }}/userGuide/syntax/extra/loadContent.html#fragment">
This is loaded from a .html file
</popover>
</p>
<p>
<popover header="From a MarkDown file" src="{{ baseUrl }}/userGuide/formattingContents.md#overview">
This is loaded from a .md file
</popover>
</p>
<h4 class="no-index">Wrap Text</h4>
<popover header="false" content="Nice!">What do you say</popover>
</variable>
Expand All @@ -76,13 +87,24 @@ This is the same <trigger for="pop:trigger_id">trigger</trigger> as last one.

****Options****

Name | Type | Default | Description
---- | ---- | ------- | ------
trigger | `String` | `hover` | How the Popover is triggered.<br>Supports: `click`, `focus`, `hover`.
header{{slot_info_trigger}} | `String` | `''` | Popover header, supports MarkDown text.
content{{slot_info_trigger}} | `String` | `''` | Popover content, supports MarkDown text.
placement | `String` | `top` | How to position the Popover.<br>Supports: `top`, `left`, `right`, `bottom`.
| Name | Type | Default | Description |
| ---------------------------- | -------- | ------- | ---------------------------------------------------------------------------------------------------------------|
| trigger | `String` | `hover` | How the Popover is triggered.<br>Supports: `click`, `focus`, `hover`. |
| header{{slot_info_trigger}} | `String` | `''` | Popover header, supports MarkDown text. |
| content{{slot_info_trigger}} | `String` | `''` | Popover content, supports MarkDown text. |
| src | `String` | | The url to the remote page to be loaded as the content of the popover.<br>Both `.md` and `.html` are accepted. |
| placement | `String` | `top` | How to position the Popover.<br>Supports: `top`, `left`, `right`, `bottom`. |

<box type="info" light>

MarkBind supports the `src` attribute, `content` attribute and `content` slot for popovers.
Usually, only one of these would be used at a time.

If multiple of these are used, MarkBind will prioritise in the following order:
1. `content` slot
1. `content` attribute
1. `src` attribute
</box>

jovyntls marked this conversation as resolved.
Show resolved Hide resolved
<span id="short" class="d-none">

Expand Down
11 changes: 11 additions & 0 deletions packages/cli/test/functional/test_site/expected/siteData.json
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,17 @@
"headings": {},
"headingKeywords": {}
},
{
"src": "testPopovers.md",
"title": "Test: Popovers",
"headings": {
"nested-panel": "Nested Panel",
"normal-panel-content-heading": "Normal panel content heading",
"some-heading": "Some heading",
"some-heading-2": "Some heading"
},
"headingKeywords": {}
},
{
"src": "testPopoverTrigger.md",
"title": "Popover initiated by trigger should honor trigger attribute",
Expand Down
338 changes: 338 additions & 0 deletions packages/cli/test/functional/test_site/expected/testPopovers.html

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions packages/cli/test/functional/test_site/site.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@
"src": "testIncludeMultipleModals.md",
"title": "Multiple inclusions of a modal should be supported"
},
{
"src": "testPopovers.md",
"title": "Test: Popovers"
},
{
"src": "testPopoverTrigger.md",
"title": "Popover initiated by trigger should honor trigger attribute"
Expand Down
115 changes: 115 additions & 0 deletions packages/cli/test/functional/test_site/testPopovers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
**Popover with attributes**
jovyntls marked this conversation as resolved.
Show resolved Hide resolved

<popover header="Correct header" content="Correct content">
Hover popover
</popover>

<br>

<popover header="Correct header" content="Correct content" trigger="click">
Click popover
</popover>

<br>

**Popover with slots**

<popover>
<span slot="content">Correct content</span>
Hover popover
</popover>

<br>

<popover trigger="click">
<span slot="header">Correct header</span>
<span slot="content">Correct content</span>
Click popover
</popover>

<br>

**Popover with slots overriding attributes**

<popover header="Correct header" content="Should not appear: Overwritten content">
<span slot="content">Correct content</span>
Hover popover
</popover>

<br>

<popover header="Should not appear: Overwritten header" content="Should not appear: Overwritten content" trigger="click">
<span slot="header">Correct header</span>
<span slot="content">Correct content</span>
Click popover
</popover>

<br>

**Popover with src attribute**

<popover header="Correct header" src="{{ baseUrl }}/test_md_fragment.md">
src from a .md file with absolute links
</popover>

<br>

<popover header="Correct header" src="./test_md_fragment.md">
src from a .md file with relative links
</popover>

<br>

<popover header="Correct header" src="{{ baseUrl }}/testInclude.html">
src from a .html file with absolute links
</popover>

<br>

<popover header="Correct header" src="./testInclude.html">
src from a .html file with relative links
</popover>

<br>

<popover src="{{ baseUrl }}/contentFragmentToInclude.md#fragment">
src with a fragment
</popover>

<br>

<popover src="{{ baseUrl }}/testPanels/NestedPanel.md">
<span slot="header">Reactive content</span>
src containing reactive content
</popover>

<br>

**Popover contents should use the priority of content slot > content attribute > src attribute**

<popover header="Content slot > content attrib > src attrib" src="{{ baseUrl }}/test_md_fragment.md" content="Correct content">
Content attribute overrides src attribute
</popover>

<br>

<popover header="Content slot > content attrib > src attrib" src="{{ baseUrl }}/test_md_fragment.md" content="This should be overwritten by slot">
<span slot="content">Correct content</span>
Content slot overrides content attribute overrides src attribute
</popover>

<br>

<popover header="Content slot > content attrib > src attrib" src="{{ baseUrl }}/test_md_fragment.md">
<span slot="content">Correct content</span>
Content slot overrides content attribute overrides src attribute
</popover>

<br>

**URLs are not valid src**

<popover header="URLs are not valid" src="http://www.shouldnotwork.com">
URLs should not be valid
</popover>

Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
</template>
<button class="btn btn-secondary">Trigger should not have `algolia-no-index` class</button>
</popover>
<popover placement="top"><template #header>Title</template><template #content>Content should have <code class="hljs inline no-lang" v-pre>algolia-no-index</code> class</template>
<popover placement="top"><template #content>Content should have <code class="hljs inline no-lang" v-pre>algolia-no-index</code> class</template><template #header>Title</template>
<button class="btn btn-secondary">Trigger should not have `algolia-no-index` class</button>
</popover>
<p><strong>Tooltip content should have algolia-no-index class</strong></p>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 43 additions & 2 deletions packages/core/src/html/MdAttributeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,50 @@ class MdAttributeRenderer {
delete node.attribs[attribute];
}

processPopover(node) {
/**
* Checks if the node has both the given slot and the given attribute,
* deleting the attribute and logging a warning if both the slot and attribute exist.
* @param node Element to process
* @param attribute Attribute name to process
* @param slotName Name attribute of the <slot> element to insert, which defaults to the attribute name
* @returns {boolean} whether the node has both the slot and attribute
*/
// eslint-disable-next-line class-methods-use-this
hasSlotOverridingAttribute(node, attribute, slotName = attribute) {
const hasNamedSlot = node.children
&& node.children.some(child => getVslotShorthandName(child) === slotName);
if (!hasNamedSlot) {
return false;
}

// If the slot is present, remove the attribute as the attribute has no effect.
const hasAttribute = _.has(node.attribs, attribute);
if (hasAttribute) {
logger.warn(`${node.name} has a ${slotName} slot, '${attribute}' attribute has no effect.`);
delete node.attribs[attribute];
}

return hasAttribute;
}

processPopoverAttributes(node) {
if (!this.hasSlotOverridingAttribute(node, 'header')) {
this.processAttributeWithoutOverride(node, 'header', true);
}

// Warn if there is a content slot overriding the attributes 'content' or 'src'
const hasSlotAndContentAttribute = this.hasSlotOverridingAttribute(node, 'content', 'content');
const hasSlotAndSrcAttribute = this.hasSlotOverridingAttribute(node, 'src', 'content');
if (hasSlotAndContentAttribute || hasSlotAndSrcAttribute) {
return;
}

if (_.has(node.attribs, 'content') && _.has(node.attribs, 'src')) {
logger.warn(`${node.name} has a 'content' attribute, 'src' attribute has no effect.`);
delete node.attribs.src;
}

this.processAttributeWithoutOverride(node, 'content', true);
this.processAttributeWithoutOverride(node, 'header', true);
}

processTooltip(node) {
Expand Down
Loading