[FIX] web: fix popover in dialog with useAutofocus

**Before this commit**
Open within a dialog a popover that has a child
using the useAutofocus hook.
As the dialog has become the UI active element,
and as the popover's element tree is a sibling
of the dialog element (since both use the overlay
service): the autofocus mechanism wanted by the
popover's child does not work at all.

**After this commit**
The popovers now can also become the UI active
element, leading to the above use case to work properly.

closes odoo/odoo#140885

Signed-off-by: Florent Dardenne (dafl) <dafl@odoo.com>
This commit is contained in:
Bruno Boi
2023-11-04 16:38:53 +00:00
parent 003609b8d2
commit f9753a3e71
5 changed files with 82 additions and 17 deletions
@@ -27,6 +27,7 @@
<t t-set="arInfo" t-value="getActiveRangeInfo(itemInfo)" />
<button
class="o_date_item_cell o_datetime_button o_center o_cell_md btn p-1 border-0 fw-normal"
tabindex="-1"
t-att-class="{
'o_selected': arInfo.isSelected,
o_select_start: arInfo.isSelectStart,
@@ -97,6 +98,7 @@
<t t-set="arInfo" t-value="getActiveRangeInfo(itemInfo)" />
<button
class="o_date_item_cell o_datetime_button btn o_center o_cell_lg btn p-1 border-0"
tabindex="-1"
t-att-class="{
'o_selected': arInfo.isSelected,
o_select_start: arInfo.isSelectStart,
@@ -120,6 +122,7 @@
<!-- Requires: { unitIndex: number, unitList: [any, string][], timeValue_index: number } -->
<select
class="o_time_picker_select form-control form-control-sm w-auto"
tabindex="-1"
t-model="timeValue[unitIndex]"
t-on-change="() => this.selectTime(timeValue_index)"
>
@@ -142,12 +145,14 @@
>
<nav class="o_datetime_picker_header btn-group">
<button class="o_previous btn btn-light flex-grow-0"
tabindex="-1"
t-att-class="isLastPrecisionLevel ? 'order-1 btn-sm px-1' : ''" t-on-click="previous">
<i class="oi oi-chevron-left" t-att-title="activePrecisionLevel.prevTitle" />
</button>
<button
class="o_zoom_out o_datetime_button btn btn-light d-flex align-items-center px-0 text-truncate"
t-att-class="isLastPrecisionLevel ? 'pe-none text-start' : 'justify-content-around'"
tabindex="-1"
t-att-title="!isLastPrecisionLevel and activePrecisionLevel.mainTitle"
t-on-click="zoomOut"
>
@@ -163,6 +168,7 @@
</button>
<button class="o_next btn btn-light flex-grow-0"
t-att-class="isLastPrecisionLevel ? 'order-2 btn-sm px-1' : ''"
tabindex="-1"
t-on-click="next">
<i class="oi oi-chevron-right" t-att-title="activePrecisionLevel.nextTitle" />
</button>
@@ -6,6 +6,7 @@
<t t-if="props.pickerProps.type === 'datetime' or Array.isArray(props.pickerProps.value)">
<button
class="o_apply btn btn-primary btn-sm w-100 w-md-auto d-flex align-items-center justify-content-center gap-1"
tabindex="-1"
t-on-click="props.close"
>
<i class="fa fa-check" />
+13 -15
View File
@@ -1,28 +1,26 @@
/** @odoo-module **/
import { Component } from "@odoo/owl";
import { useForwardRefToParent } from "../utils/hooks";
import { useForwardRefToParent } from "@web/core/utils/hooks";
import { usePosition } from "@web/core/position_hook";
import { useActiveElement } from "@web/core/ui/ui_service";
export class Popover extends Component {
static animationTime = 200;
setup() {
useActiveElement("ref");
useForwardRefToParent("ref");
this.shouldAnimate = this.props.animation;
this.position = usePosition(
"ref",
() => this.props.target,
{
onPositioned: (el, solution) => {
(this.props.onPositioned || this.onPositioned.bind(this))(el, solution);
if (this.props.fixedPosition) {
// Prevent further positioning updates if fixed position is wanted
this.position.lock();
}
},
position: this.props.position,
}
);
this.position = usePosition("ref", () => this.props.target, {
onPositioned: (el, solution) => {
(this.props.onPositioned || this.onPositioned.bind(this))(el, solution);
if (this.props.fixedPosition) {
// Prevent further positioning updates if fixed position is wanted
this.position.lock();
}
},
position: this.props.position,
});
}
onPositioned(el, { direction, variant }) {
const position = `${direction[0]}${variant[0]}`;
@@ -18,7 +18,9 @@ import {
patchWithCleanup,
} from "../helpers/utils";
import { Dialog } from "../../src/core/dialog/dialog";
import { popoverService } from "@web/core/popover/popover_service";
import { usePopover } from "@web/core/popover/popover_hook";
import { useAutofocus } from "@web/core/utils/hooks";
import { Component, onMounted, xml } from "@odoo/owl";
let env;
@@ -143,6 +145,44 @@ QUnit.test("multiple dialogs can become the UI active element", async (assert) =
assert.strictEqual(dialogModal, env.services.ui.activeElement);
});
QUnit.test("a popover with an autofocus child can become the UI active element", async (assert) => {
class TestPopover extends Component {
static template = xml`<input type="text" t-ref="autofocus" />`;
setup() {
useAutofocus();
}
}
class CustomDialog extends Component {
static components = { Dialog };
static template = xml`<Dialog title="props.title">
<button class="btn test" t-on-click="showPopover">show</button>
</Dialog>`;
setup() {
this.popover = usePopover(TestPopover);
}
showPopover(event) {
this.popover.open(event.target, {});
}
}
serviceRegistry.add("popover", popoverService);
await nextTick(); // wait for the popover service to be started
await mount(PseudoWebClient, target, { env });
assert.strictEqual(env.services.ui.activeElement, document);
assert.strictEqual(document.activeElement, document.body);
env.services.dialog.add(CustomDialog, { title: "Hello" });
await nextTick();
const dialogModal = target.querySelector(".o_dialog:not(.o_inactive_modal) .modal");
assert.strictEqual(env.services.ui.activeElement, dialogModal);
assert.strictEqual(document.activeElement, dialogModal.querySelector(".btn.o-default-button"));
await click(dialogModal, ".btn.test");
const popover = target.querySelector(".o_popover");
const input = popover.querySelector("input");
assert.strictEqual(env.services.ui.activeElement, popover);
assert.strictEqual(document.activeElement, input);
});
QUnit.test("Interactions between multiple dialogs", async (assert) => {
assert.expect(10);
function activity(modals) {
@@ -4,7 +4,11 @@ import { Popover } from "@web/core/popover/popover";
import { usePosition } from "@web/core/position_hook";
import { registerCleanup } from "../../helpers/cleanup";
import { getFixture, makeDeferred, mount, nextTick, triggerEvent } from "../../helpers/utils";
import { makeTestEnv } from "../../helpers/mock_env";
import { registry } from "@web/core/registry";
import { uiService } from "@web/core/ui/ui_service";
let env;
let fixture;
let popoverTarget;
@@ -19,11 +23,15 @@ QUnit.module("Popover", {
registerCleanup(() => {
popoverTarget.remove();
});
registry.category("services").add("ui", uiService);
env = await makeTestEnv();
},
});
QUnit.test("popover can have custom class", async (assert) => {
await mount(Popover, fixture, {
env,
props: { target: popoverTarget, class: "custom-popover" },
});
@@ -32,6 +40,7 @@ QUnit.test("popover can have custom class", async (assert) => {
QUnit.test("popover can have more than one custom class", async (assert) => {
await mount(Popover, fixture, {
env,
props: { target: popoverTarget, class: "custom-popover popover-custom" },
});
@@ -46,6 +55,7 @@ QUnit.test("popover is rendered nearby target (default)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget },
});
});
@@ -58,6 +68,7 @@ QUnit.test("popover is rendered nearby target (bottom)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "bottom" },
});
});
@@ -70,6 +81,7 @@ QUnit.test("popover is rendered nearby target (top)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "top" },
});
});
@@ -82,6 +94,7 @@ QUnit.test("popover is rendered nearby target (left)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "left" },
});
});
@@ -94,6 +107,7 @@ QUnit.test("popover is rendered nearby target (right)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "right" },
});
});
@@ -106,6 +120,7 @@ QUnit.test("popover is rendered nearby target (bottom-start)", async (assert) =>
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "bottom-start" },
});
});
@@ -118,6 +133,7 @@ QUnit.test("popover is rendered nearby target (bottom-middle)", async (assert) =
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "bottom-middle" },
});
});
@@ -130,6 +146,7 @@ QUnit.test("popover is rendered nearby target (bottom-end)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "bottom-end" },
});
});
@@ -142,6 +159,7 @@ QUnit.test("popover is rendered nearby target (bottom-fit)", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget, position: "bottom-fit" },
});
});
@@ -190,7 +208,7 @@ QUnit.test("reposition popover should properly change classNames", async (assert
}
};
await mount(TestPopover, container, { props: { target: popoverTarget } });
await mount(TestPopover, container, { env, props: { target: popoverTarget } });
const popover = container.querySelector("[role=tooltip]");
const arrow = popover.firstElementChild;
@@ -234,6 +252,7 @@ QUnit.test("within iframe", async (assert) => {
popoverTarget = iframe.contentDocument.getElementById("target");
await mount(TestPopover, fixture, {
env,
props: { target: popoverTarget },
});
assert.verifySteps(["bottom"]);
@@ -287,6 +306,7 @@ QUnit.test("popover fixed position", async (assert) => {
}
};
await mount(TestPopover, fixture, {
env,
props: { target: container, position: "bottom-fit", fixedPosition: true },
});