From cad8fbcbf57072ade39eb10b5b8950bae7c9dd17 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Thu, 13 Apr 2023 11:32:28 +0000 Subject: [PATCH] [IMP] web: allow all positions in popovers **Before this commit** Only four positions authorized for popovers: - bottom, - left, - top, - right. These four positions are by default with the "middle" variant. But the positioning hook could replace the variant with "start" or "end" and furthermore the popover already handle those cases to properly place its arrow. So it does make sense to authorize all the possible variants as well. **After this commit** All the following position are available to popovers: - {bottom|left|top|right}-{start|middle|end|fit}, - {bottom|left|top|right} (same as {bottom|left|top|right}-middle). Part-of: odoo/odoo#118066 --- addons/web/static/src/core/popover/popover.js | 12 +++- .../src/core/popover/popover_service.js | 4 +- .../tests/core/popover/popover_tests.js | 64 +++++++++++++++++-- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/addons/web/static/src/core/popover/popover.js b/addons/web/static/src/core/popover/popover.js index aa0d7722cd7..d0ceea0cb5e 100644 --- a/addons/web/static/src/core/popover/popover.js +++ b/addons/web/static/src/core/popover/popover.js @@ -39,10 +39,14 @@ export class Popover extends Component { switch (position) { case "tm": // top-middle case "bm": // bottom-middle + case "tf": // top-fit + case "bf": // bottom-fit arrowEl.classList.add("start-0", "end-0", "mx-auto"); break; case "lm": // left-middle case "rm": // right-middle + case "lf": // left-fit + case "rf": // right-fit arrowEl.classList.add("top-0", "bottom-0", "my-auto"); break; case "ts": // top-start @@ -81,7 +85,13 @@ Popover.props = { }, position: { type: String, - validate: (p) => ["top", "bottom", "left", "right"].includes(p), + validate: (p) => { + const [d, v = "middle"] = p.split("-"); + return ( + ["top", "bottom", "left", "right"].includes(d) && + ["start", "middle", "end", "fit"].includes(v) + ); + }, optional: true, }, onPositioned: { diff --git a/addons/web/static/src/core/popover/popover_service.js b/addons/web/static/src/core/popover/popover_service.js index 99ebbf617ee..b53ce3f214e 100644 --- a/addons/web/static/src/core/popover/popover_service.js +++ b/addons/web/static/src/core/popover/popover_service.js @@ -6,10 +6,10 @@ import { PopoverController } from "./popover_controller"; /** * @typedef {{ - * closeOnClickAway?: boolean || (target: HTMLElement) => boolean; + * closeOnClickAway?: boolean | (target: HTMLElement) => boolean; * onClose?: () => void; * popoverClass?: string; - * position?: import("@web/core/position_hook").Direction; + * position?: import("@web/core/position_hook").Options["position"]; * onPositioned?: import("@web/core/position_hook").PositionEventHandler; * }} PopoverServiceAddOptions */ diff --git a/addons/web/static/tests/core/popover/popover_tests.js b/addons/web/static/tests/core/popover/popover_tests.js index 35e730608fb..b158dd1b52f 100644 --- a/addons/web/static/tests/core/popover/popover_tests.js +++ b/addons/web/static/tests/core/popover/popover_tests.js @@ -39,10 +39,10 @@ QUnit.test("popover can have more than one custom class", async (assert) => { }); QUnit.test("popover is rendered nearby target (default)", async (assert) => { - assert.expect(1); const TestPopover = class extends Popover { - onPositioned(el, { direction }) { + onPositioned(el, { direction, variant }) { assert.equal(direction, "bottom"); + assert.equal(variant, "middle"); } }; await mount(TestPopover, fixture, { @@ -52,8 +52,9 @@ QUnit.test("popover is rendered nearby target (default)", async (assert) => { QUnit.test("popover is rendered nearby target (bottom)", async (assert) => { const TestPopover = class extends Popover { - onPositioned(el, { direction }) { + onPositioned(el, { direction, variant }) { assert.equal(direction, "bottom"); + assert.equal(variant, "middle"); } }; await mount(TestPopover, fixture, { @@ -63,8 +64,9 @@ QUnit.test("popover is rendered nearby target (bottom)", async (assert) => { QUnit.test("popover is rendered nearby target (top)", async (assert) => { const TestPopover = class extends Popover { - onPositioned(el, { direction }) { + onPositioned(el, { direction, variant }) { assert.equal(direction, "top"); + assert.equal(variant, "middle"); } }; await mount(TestPopover, fixture, { @@ -74,8 +76,9 @@ QUnit.test("popover is rendered nearby target (top)", async (assert) => { QUnit.test("popover is rendered nearby target (left)", async (assert) => { const TestPopover = class extends Popover { - onPositioned(el, { direction }) { + onPositioned(el, { direction, variant }) { assert.equal(direction, "left"); + assert.equal(variant, "middle"); } }; await mount(TestPopover, fixture, { @@ -85,8 +88,9 @@ QUnit.test("popover is rendered nearby target (left)", async (assert) => { QUnit.test("popover is rendered nearby target (right)", async (assert) => { const TestPopover = class extends Popover { - onPositioned(el, { direction }) { + onPositioned(el, { direction, variant }) { assert.equal(direction, "right"); + assert.equal(variant, "middle"); } }; await mount(TestPopover, fixture, { @@ -94,6 +98,54 @@ QUnit.test("popover is rendered nearby target (right)", async (assert) => { }); }); +QUnit.test("popover is rendered nearby target (bottom-start)", async (assert) => { + const TestPopover = class extends Popover { + onPositioned(el, { direction, variant }) { + assert.equal(direction, "bottom"); + assert.equal(variant, "start"); + } + }; + await mount(TestPopover, fixture, { + props: { target: popoverTarget, position: "bottom-start" }, + }); +}); + +QUnit.test("popover is rendered nearby target (bottom-middle)", async (assert) => { + const TestPopover = class extends Popover { + onPositioned(el, { direction, variant }) { + assert.equal(direction, "bottom"); + assert.equal(variant, "middle"); + } + }; + await mount(TestPopover, fixture, { + props: { target: popoverTarget, position: "bottom-middle" }, + }); +}); + +QUnit.test("popover is rendered nearby target (bottom-end)", async (assert) => { + const TestPopover = class extends Popover { + onPositioned(el, { direction, variant }) { + assert.equal(direction, "bottom"); + assert.equal(variant, "end"); + } + }; + await mount(TestPopover, fixture, { + props: { target: popoverTarget, position: "bottom-end" }, + }); +}); + +QUnit.test("popover is rendered nearby target (bottom-fit)", async (assert) => { + const TestPopover = class extends Popover { + onPositioned(el, { direction, variant }) { + assert.equal(direction, "bottom"); + assert.equal(variant, "fit"); + } + }; + await mount(TestPopover, fixture, { + props: { target: popoverTarget, position: "bottom-fit" }, + }); +}); + QUnit.test("reposition popover should properly change classNames", async (assert) => { // Force some style, to make this test independent of screen size const container = document.createElement("div");