Commit db00525b authored by Vitaly Slobodin's avatar Vitaly Slobodin

Merge branch 'remove-tooltip-directive' into 'master'

Remove BS tooltip directive wrapper

See merge request gitlab-org/gitlab!54615
parents cb6d5041 ff2c0527
<script>
import { GlTooltipDirective as GlTooltip } from '@gitlab/ui';
import { isFunction } from 'lodash';
import { hasHorizontalOverflow } from '~/lib/utils/dom_utils';
import tooltip from '../directives/tooltip';
export default {
directives: {
tooltip,
GlTooltip,
},
props: {
title: {
......@@ -59,9 +59,8 @@ export default {
<template>
<span
v-if="showTooltip"
v-tooltip
v-gl-tooltip="{ placement }"
:title="title"
:data-placement="placement"
class="js-show-tooltip gl-min-w-0"
>
<slot></slot>
......
import $ from 'jquery';
import '~/commons/bootstrap';
import { parseBoolean } from '~/lib/utils/common_utils';
export default {
bind(el) {
const glTooltipDelay = localStorage.getItem('gl-tooltip-delay');
const delay = glTooltipDelay ? JSON.parse(glTooltipDelay) : 0;
$(el).tooltip({
trigger: 'hover',
delay,
// By default, sanitize is run even if there is no `html` or `template` present
// so let's optimize to only run this when necessary.
// https://github.com/twbs/bootstrap/blob/c5966de27395a407f9a3d20d0eb2ff8e8fb7b564/js/src/tooltip.js#L716
sanitize: parseBoolean(el.dataset.html) || Boolean(el.dataset.template),
});
},
componentUpdated(el) {
$(el).tooltip('_fixTitle');
// update visible tooltips
const tooltipInstance = $(el).data('bs.tooltip');
const tip = tooltipInstance.getTipElement();
tooltipInstance.setElementContent(
$(tip.querySelectorAll('.tooltip-inner')),
tooltipInstance.getTitle(),
);
},
unbind(el) {
$(el).tooltip('dispose');
},
};
......@@ -42,7 +42,6 @@
@import 'framework/notes';
@import 'framework/tabs';
@import 'framework/timeline';
@import 'framework/tooltips';
@import 'framework/toggle';
@import 'framework/typography';
@import 'framework/zen';
......
......@@ -283,12 +283,6 @@ ul.indent-list {
to { transform: rotate(360deg); }
}
.namespace-title {
.tooltip-inner {
max-width: 350px;
}
}
.horizontal-list {
padding-left: 0;
list-style: none;
......
.tooltip-inner {
font-size: $gl-font-size-small;
border-radius: $border-radius-default;
line-height: $gl-line-height;
font-weight: $gl-font-weight-normal;
}
......@@ -432,41 +432,7 @@ must be unique. It's advised to use `kebab-case` namespaces.
Useful links:
1. [`key`](https://vuejs.org/v2/guide/list.html#key)
1. [Vue Style Guide: Keyed v-for](https://vuejs.org/v2/style-guide/#Keyed-v-for-essential )
## Vue and Bootstrap
1. Tooltips: Do not rely on `has-tooltip` class name for Vue components
```html
// bad
<span
class="has-tooltip"
title="Some tooltip text">
Text
</span>
// good
<span
v-tooltip
title="Some tooltip text">
Text
</span>
```
1. Tooltips: When using a tooltip, include the tooltip directive, `./app/assets/javascripts/vue_shared/directives/tooltip.js`
1. Don't change `data-original-title`.
```html
// bad
<span data-original-title="tooltip text">Foo</span>
// good
<span title="tooltip text">Foo</span>
$('span').tooltip('_fixTitle');
```
1. [Vue Style Guide: Keyed v-for](https://vuejs.org/v2/style-guide/#Keyed-v-for-essential)
## Vue testing
......
......@@ -84,10 +84,6 @@
}
}
.tooltip .tooltip-inner .milestone-date-range {
color: var(--gray-400, $gray-400);
}
.new-epic-form .md-area.gfm-form {
@include gl-rounded-base;
@include gl-border-none;
......
......@@ -11,6 +11,15 @@ jest.mock('~/lib/utils/dom_utils', () => ({
throw new Error('this needs to be mocked');
}),
}));
jest.mock('@gitlab/ui', () => ({
GlTooltipDirective: {
bind(el, binding) {
el.classList.add('gl-tooltip');
el.setAttribute('data-original-title', el.title);
el.dataset.placement = binding.value.placement;
},
},
}));
describe('TooltipOnTruncate component', () => {
let wrapper;
......@@ -52,7 +61,7 @@ describe('TooltipOnTruncate component', () => {
wrapper = parent.find(TooltipOnTruncate);
};
const hasTooltip = () => wrapper.classes('js-show-tooltip');
const hasTooltip = () => wrapper.classes('gl-tooltip');
afterEach(() => {
wrapper.destroy();
......
import { mount } from '@vue/test-utils';
import $ from 'jquery';
import { escape } from 'lodash';
import tooltip from '~/vue_shared/directives/tooltip';
const DEFAULT_TOOLTIP_TEMPLATE = '<div v-tooltip :title="tooltip"></div>';
const HTML_TOOLTIP_TEMPLATE = '<div v-tooltip data-html="true" :title="tooltip"></div>';
describe('Tooltip directive', () => {
let wrapper;
function createTooltipContainer({
template = DEFAULT_TOOLTIP_TEMPLATE,
text = 'some text',
} = {}) {
wrapper = mount(
{
directives: { tooltip },
data: () => ({ tooltip: text }),
template,
},
{ attachTo: document.body },
);
}
async function showTooltip() {
$(wrapper.vm.$el).tooltip('show');
jest.runOnlyPendingTimers();
await wrapper.vm.$nextTick();
}
function findTooltipInnerHtml() {
return document.querySelector('.tooltip-inner').innerHTML;
}
function findTooltipHtml() {
return document.querySelector('.tooltip').innerHTML;
}
afterEach(() => {
wrapper.destroy();
wrapper = null;
});
describe('with a single tooltip', () => {
it('should have tooltip plugin applied', () => {
createTooltipContainer();
expect($(wrapper.vm.$el).data('bs.tooltip')).toBeDefined();
});
it('displays the title as tooltip', () => {
createTooltipContainer();
$(wrapper.vm.$el).tooltip('show');
jest.runOnlyPendingTimers();
const tooltipElement = document.querySelector('.tooltip-inner');
expect(tooltipElement.textContent).toContain('some text');
});
it.each`
condition | template | sanitize
${'does not contain any html'} | ${DEFAULT_TOOLTIP_TEMPLATE} | ${false}
${'contains html'} | ${HTML_TOOLTIP_TEMPLATE} | ${true}
`('passes sanitize=$sanitize if the tooltip $condition', ({ template, sanitize }) => {
createTooltipContainer({ template });
expect($(wrapper.vm.$el).data('bs.tooltip').config.sanitize).toEqual(sanitize);
});
it('updates a visible tooltip', async () => {
createTooltipContainer();
$(wrapper.vm.$el).tooltip('show');
jest.runOnlyPendingTimers();
const tooltipElement = document.querySelector('.tooltip-inner');
wrapper.vm.tooltip = 'other text';
jest.runOnlyPendingTimers();
await wrapper.vm.$nextTick();
expect(tooltipElement.textContent).toContain('other text');
});
describe('tooltip sanitization', () => {
it('reads tooltip content as text if data-html is not passed', async () => {
createTooltipContainer({ text: 'sample text<script>alert("XSS!!")</script>' });
await showTooltip();
const result = findTooltipInnerHtml();
expect(result).toEqual('sample text&lt;script&gt;alert("XSS!!")&lt;/script&gt;');
});
it('sanitizes tooltip if data-html is passed', async () => {
createTooltipContainer({
template: HTML_TOOLTIP_TEMPLATE,
text: 'sample text<script>alert("XSS!!")</script>',
});
await showTooltip();
const result = findTooltipInnerHtml();
expect(result).toEqual('sample text');
expect(result).not.toContain('XSS!!');
});
it('sanitizes tooltip if data-template is passed', async () => {
const tooltipTemplate = escape(
'<div class="tooltip" role="tooltip"><div onclick="alert(\'XSS!\')" class="arrow"></div><div class="tooltip-inner"></div></div>',
);
createTooltipContainer({
template: `<div v-tooltip :title="tooltip" data-html="false" data-template="${tooltipTemplate}"></div>`,
});
await showTooltip();
const result = findTooltipHtml();
expect(result).toEqual(
// objectionable element is removed
'<div class="arrow"></div><div class="tooltip-inner">some text</div>',
);
expect(result).not.toContain('XSS!!');
});
});
});
describe('with multiple tooltips', () => {
beforeEach(() => {
createTooltipContainer({
template: `
<div>
<div
v-tooltip
class="js-look-for-tooltip"
title="foo">
</div>
<div
v-tooltip
title="bar">
</div>
</div>
`,
});
});
it('should have tooltip plugin applied to all instances', () => {
expect($(wrapper.vm.$el).find('.js-look-for-tooltip').data('bs.tooltip')).toBeDefined();
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment