Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ snapshots["vaadin-avatar-group default"] =
<div
id="vaadin-tooltip-0"
role="tooltip"
slot="sr-label"
slot="overlay"
>
</div>
</vaadin-tooltip>
Expand Down Expand Up @@ -55,7 +55,7 @@ snapshots["vaadin-avatar-group items"] =
<div
id="vaadin-tooltip-2"
role="tooltip"
slot="sr-label"
slot="overlay"
>
YY
</div>
Expand All @@ -77,7 +77,7 @@ snapshots["vaadin-avatar-group items"] =
<div
id="vaadin-tooltip-3"
role="tooltip"
slot="sr-label"
slot="overlay"
>
Tomi Virkki
</div>
Expand All @@ -101,7 +101,7 @@ snapshots["vaadin-avatar-group items"] =
<div
id="vaadin-tooltip-1"
role="tooltip"
slot="sr-label"
slot="overlay"
>
YY
Tomi Virkki
Expand Down Expand Up @@ -139,7 +139,7 @@ snapshots["vaadin-avatar-group theme"] =
<div
id="vaadin-tooltip-5"
role="tooltip"
slot="sr-label"
slot="overlay"
>
YY
</div>
Expand All @@ -162,7 +162,7 @@ snapshots["vaadin-avatar-group theme"] =
<div
id="vaadin-tooltip-6"
role="tooltip"
slot="sr-label"
slot="overlay"
>
Tomi Virkki
</div>
Expand All @@ -187,7 +187,7 @@ snapshots["vaadin-avatar-group theme"] =
<div
id="vaadin-tooltip-4"
role="tooltip"
slot="sr-label"
slot="overlay"
>
YY
Tomi Virkki
Expand Down Expand Up @@ -259,7 +259,7 @@ snapshots["vaadin-avatar-group opened default"] =
<div
id="vaadin-tooltip-8"
role="tooltip"
slot="sr-label"
slot="overlay"
>
Abc Def
</div>
Expand All @@ -281,7 +281,7 @@ snapshots["vaadin-avatar-group opened default"] =
<div
id="vaadin-tooltip-9"
role="tooltip"
slot="sr-label"
slot="overlay"
>
Ghi Jkl
</div>
Expand All @@ -305,7 +305,7 @@ snapshots["vaadin-avatar-group opened default"] =
<div
id="vaadin-tooltip-7"
role="tooltip"
slot="sr-label"
slot="overlay"
>
Mno Pqr
Stu Vwx
Expand Down
1 change: 1 addition & 0 deletions packages/menu-bar/src/vaadin-menu-bar-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ export const MenuBarMixin = (superClass) =>
_hideTooltip(immediate) {
const tooltip = this._tooltipController && this._tooltipController.node;
if (tooltip) {
this._tooltipController.setContext({ item: null });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, tooltip was preserving the text after closing, which is rather unexpected. Tests didn't catch that because the IT assertion returned null if no vaadin-tooltip-overlay was found in the document.

tooltip._stateController.close(immediate);
}
}
Expand Down
69 changes: 13 additions & 56 deletions packages/tooltip/src/vaadin-tooltip-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ export const TooltipMixin = (superClass) =>
*/
text: {
type: String,
observer: '__textChanged',
},

/**
Expand Down Expand Up @@ -366,26 +365,9 @@ export const TooltipMixin = (superClass) =>
type: Boolean,
sync: true,
},

/** @private */
_srLabel: {
type: Object,
},

/** @private */
_overlayContent: {
type: String,
},
};
}

static get observers() {
return [
'__generatorChanged(_overlayElement, generator, context)',
'__updateSrLabelText(_srLabel, _overlayContent)',
];
}

/**
* Sets the default focus delay to be used by all tooltip instances,
* except for those that have focus delay configured using property.
Expand Down Expand Up @@ -420,7 +402,6 @@ export const TooltipMixin = (superClass) =>
super();

this._uniqueId = `vaadin-tooltip-${generateUniqueId()}`;
this._renderer = this.__tooltipRenderer.bind(this);

this.__onFocusin = this.__onFocusin.bind(this);
this.__onFocusout = this.__onFocusout.bind(this);
Expand Down Expand Up @@ -467,19 +448,24 @@ export const TooltipMixin = (superClass) =>

this._overlayElement = this.$.overlay;

this._srLabelController = new SlotController(this, 'sr-label', 'div', {
this.__contentController = new SlotController(this, 'overlay', 'div', {
initializer: (element) => {
element.id = this._uniqueId;
element.setAttribute('role', 'tooltip');
this._srLabel = element;
this.__contentNode = element;
},
});
this.addController(this._srLabelController);
this.addController(this.__contentController);
}

/** @private */
__computeOpened(manual, opened, autoOpened, connected) {
return connected && (manual ? opened : autoOpened);
/** @protected */
updated(props) {
super.updated(props);

if (props.has('text') || props.has('generator') || props.has('context')) {
this.__updateContent();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this method. It mainly exists because of an ugly internal hack which is likely used in some customer project and relies on patching _renderer. Maybe we can just break that assumption and inline setting textContent here in updated().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't really hurt to keep it.

this.$.overlay.toggleAttribute('hidden', this.__contentNode.textContent.trim() === '');
}
}

/** @private */
Expand Down Expand Up @@ -688,18 +674,8 @@ export const TooltipMixin = (superClass) =>
}

/** @private */
__textChanged(text, oldText) {
if (this._overlayElement && (text || oldText)) {
this._overlayElement.requestContentUpdate();
}
}

/** @private */
__tooltipRenderer(root) {
root.textContent = typeof this.generator === 'function' ? this.generator(this.context) : this.text;

// Update the sr-only label text content
this._overlayContent = root.textContent;
__updateContent() {
this.__contentNode.textContent = typeof this.generator === 'function' ? this.generator(this.context) : this.text;
}

/** @private */
Expand All @@ -723,23 +699,4 @@ export const TooltipMixin = (superClass) =>
});
}
}

/** @private */
__generatorChanged(overlayElement, generator, context) {
if (overlayElement) {
if (generator !== this.__oldTextGenerator || context !== this.__oldContext) {
overlayElement.requestContentUpdate();
}

this.__oldTextGenerator = generator;
this.__oldContext = context;
}
}

/** @private */
__updateSrLabelText(srLabel, textContent) {
if (srLabel) {
srLabel.textContent = textContent;
}
}
};
16 changes: 13 additions & 3 deletions packages/tooltip/src/vaadin-tooltip-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,20 @@ class TooltipOverlay extends PopoverOverlayMixin(
`;
}

requestContentUpdate() {
super.requestContentUpdate();
/**
* @protected
* @override
*/
_attachOverlay() {
this.showPopover();
}

this.toggleAttribute('hidden', this.textContent.trim() === '');
/**
* @protected
* @override
*/
_detachOverlay() {
this.hidePopover();
}
}

Expand Down
9 changes: 5 additions & 4 deletions packages/tooltip/src/vaadin-tooltip.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ export { TooltipPosition } from './vaadin-tooltip-mixin.js';
*
* ### Styling
*
* `<vaadin-tooltip>` uses `<vaadin-tooltip-overlay>` internal
* themable component as the actual visible overlay.
* The following shadow DOM parts are available for styling:
*
* See [`<vaadin-overlay>`](#/elements/vaadin-overlay) documentation
* for `<vaadin-tooltip-overlay>` parts.
* Part name | Description
* ----------- | ---------------
* `overlay` | The overlay element
* `content` | The overlay content element
*
* The following state attributes are available for styling:
*
Expand Down
17 changes: 9 additions & 8 deletions packages/tooltip/src/vaadin-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import { TooltipMixin } from './vaadin-tooltip-mixin.js';
*
* ### Styling
*
* `<vaadin-tooltip>` uses `<vaadin-tooltip-overlay>` internal
* themable component as the actual visible overlay.
* The following shadow DOM parts are available for styling:
*
* See [`<vaadin-overlay>`](#/elements/vaadin-overlay) documentation
* for `<vaadin-tooltip-overlay>` parts.
* Part name | Description
* ----------- | ---------------
* `overlay` | The overlay element
* `content` | The overlay content element
*
* The following state attributes are available for styling:
*
Expand Down Expand Up @@ -64,7 +65,7 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
static get styles() {
return css`
:host {
display: none;
display: contents;
}
`;
}
Expand All @@ -76,7 +77,7 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
return html`
<vaadin-tooltip-overlay
id="overlay"
.renderer="${this._renderer}"
popover="manual"
.owner="${this}"
theme="${ifDefined(this._theme)}"
.opened="${this._isConnected && (this.manual ? this.opened : this._autoOpened)}"
Expand All @@ -89,9 +90,9 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
@mouseenter="${this.__onOverlayMouseEnter}"
@mouseleave="${this.__onOverlayMouseLeave}"
modeless
exportparts="overlay, content"
><slot name="overlay"></slot
></vaadin-tooltip-overlay>

<slot name="sr-label"></slot>
`;
}
}
Expand Down
Loading
Loading