FIX: replaces data-tooltip usage by <DTooltip /> (#24062)

As much as possible I would like us to avoid having to go the with a global event listener on click/mouseover. For now I have removed all cases of `data-tooltip`, if we clearly identify a use case of a global event listener we might reconsider this.

The following changes are also included:
- by default tooltips won't attempt to focus first focusable element anymore
- tooltip will now use `cursor: pointer` by default
- a new service has been introduced: `InternalTooltip` which is responsible to track the current instance displayed by a `<DTooltip />`. Portal elements when replaced are not properly cleaned and I couldn't figure out a way to have a proper hook to ensure the previous `DTooltipInstance` is properly set as not expanded; this problem was very visible when using a tooltip as interactive and hovering another tooltip, which would replace the interactive tooltip as not closed.
This commit is contained in:
Joffrey JAFFEUX 2023-10-23 21:09:02 +02:00 committed by GitHub
parent e2d9117378
commit 043b4a4187
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 62 additions and 41 deletions

View File

@ -24,24 +24,27 @@
</a>
{{#if this.model.description}}
{{#if this.model.description_link}}
<a
target="_blank"
rel="noopener noreferrer"
href={{this.model.description_link}}
class="info"
data-tooltip={{this.model.description}}
>
<DTooltip
@interactive={{this.model.description_link.length}}
>
<:trigger>
{{d-icon "question-circle"}}
</a>
{{else}}
<span
class="info"
data-tooltip={{this.model.description}}
>
{{d-icon "question-circle"}}
</span>
{{/if}}
</:trigger>
<:content>
{{#if this.model.description_link}}
<a
target="_blank"
rel="noopener noreferrer"
href={{this.model.description_link}}
class="info"
>
{{this.model.description}}
</a>
{{else}}
<span>{{this.model.description}}</span>
{{/if}}
</:content>
</DTooltip>
{{/if}}
</li>
{{/unless}}

View File

@ -88,16 +88,15 @@
<td><Input @type="checkbox" @checked={{act.selected}} /></td>
<td>
<div class="scope-name">{{act.name}}</div>
<span
class="scope-tooltip"
data-tooltip={{i18n
<DTooltip
@icon="question-circle"
@content={{i18n
(concat
"admin.api.scopes.descriptions." resource "." act.key
)
class="scope-tooltip"
}}
>
{{d-icon "question-circle"}}
</span>
/>
</td>
<td>
<DButton

View File

@ -119,9 +119,9 @@
<td>{{scope.resource}}</td>
<td>
{{scope.action}}
<span
class="scope-tooltip"
data-tooltip={{i18n
<DTooltip
@icon="question-circle"
@content={{i18n
(concat
"admin.api.scopes.descriptions."
scope.resource
@ -129,9 +129,8 @@
scope.key
)
}}
>
{{d-icon "question-circle"}}
</span>
class="scope-tooltip"
/>
</td>
<td>
<DButton

View File

@ -19,7 +19,7 @@
...attributes
{{did-insert this.setupListeners}}
{{will-destroy this.cleanupListeners}}
{{trap-tab (hash preventScroll=false)}}
{{trap-tab preventScroll=false}}
>
<div class="modal-outer-container">
<div class="modal-middle-container">

View File

@ -13,8 +13,9 @@ export default class TrapTabModifier extends Modifier {
registerDestructor(this, (instance) => instance.cleanup());
}
modify(element, [options]) {
this.preventScroll = options?.preventScroll ?? true;
modify(element, _, { preventScroll, autofocus }) {
autofocus ??= true;
this.preventScroll = preventScroll ?? true;
this.orignalElement = element;
this.element = element.querySelector(".modal-inner-container") || element;
this.orignalElement.addEventListener("keydown", this.trapTab);
@ -23,7 +24,10 @@ export default class TrapTabModifier extends Modifier {
// and apply manual focus only if we don't have any autofocus element
const autofocusedElement = this.element.querySelector("[autofocus]");
if (!autofocusedElement || document.activeElement !== autofocusedElement) {
if (
autofocus &&
(!autofocusedElement || document.activeElement !== autofocusedElement)
) {
// if there's not autofocus, or the activeElement, is not the autofocusable element
// attempt to focus the first of the focusable elements or just the modal-body
// to make it possible to scroll with arrow down/up

View File

@ -21,11 +21,11 @@ module("Integration | Component | admin-report", function (hooks) {
"it has a title"
);
assert.strictEqual(
query(".header .info").getAttribute("data-tooltip"),
"New account registrations for this period",
"it has a description"
);
await click("[data-trigger]");
assert
.dom("[data-content]")
.hasText("New account registrations for this period");
assert.strictEqual(
query(

View File

@ -62,7 +62,7 @@ export default class DFloatBody extends Component {
aria-expanded={{if @instance.expanded "true" "false"}}
role={{@role}}
{{FloatKitApplyFloatingUi this.trigger this.options @instance}}
{{(if @trapTab (modifier TrapTab))}}
{{(if @trapTab (modifier TrapTab autofocus=false))}}
{{(if
this.supportsCloseOnClickOutside
(modifier FloatKitCloseOnClickOutside this.trigger @instance.close)

View File

@ -11,6 +11,7 @@ import and from "truth-helpers/helpers/and";
export default class DTooltip extends Component {
@service tooltip;
@service internalTooltip;
@tracked tooltipInstance = null;
@ -19,8 +20,9 @@ export default class DTooltip extends Component {
...this.args,
...{
listeners: true,
beforeTrigger: () => {
this.tooltip.close();
beforeTrigger: (instance) => {
this.internalTooltip.activeTooltip?.close?.();
this.internalTooltip.activeTooltip = instance;
},
},
};

View File

@ -0,0 +1,12 @@
import { tracked } from "@glimmer/tracking";
import Service from "@ember/service";
/*
This service holds the current tooltip displayed when using <DTooltip> component.
All of these tooltips share a commong portal outlet element, which means
we have to ensure we close them before their html is replaced, otherwise
we end up with a detached element in the DOM and unexpected behavior.
*/
export default class InternalTooltip extends Service {
@tracked activeTooltip;
}

View File

@ -0,0 +1 @@
export { default } from "float-kit/services/internal-tooltip";

View File

@ -23,6 +23,7 @@
&__trigger {
display: inline-flex;
cursor: pointer;
.touch & {
@include unselectable;