[FIX] web: anchor scroll crash
This commit fixes some issues happening when scrolling with an anchor in Odoo. When the anchor was contained inside of a notebook and no scrollable was found, a crash happens. This fix improves the behavior in notebooks by showing the correct pane before scrolling in form renderer. The scrollTo util has been improved to remove duplicate lines and handle multiple situations (0, one or more scrollables). Tests haven been written to test such behaviors. closes odoo/odoo#79935 X-original-commit: 4c2b807d1cb32e52a1b1fb5e6c80fb7e933940bc Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
This commit is contained in:
luvi
@@ -5,7 +5,7 @@ import { registry } from "./registry";
|
||||
import { scrollTo } from "./utils/scrolling";
|
||||
|
||||
export const scrollerService = {
|
||||
start() {
|
||||
start(env) {
|
||||
/**
|
||||
* Listen to click event to allow having links with href towards an anchor.
|
||||
* Since odoo use hashtag to represent the current state of the view, we can't
|
||||
@@ -41,7 +41,15 @@ export const scrollerService = {
|
||||
} catch (e) {
|
||||
// Invalid selector: not an anchor anyway
|
||||
}
|
||||
if (matchingEl) {
|
||||
const triggerEv = new CustomEvent("anchor-link-clicked", {
|
||||
detail: {
|
||||
element: matchingEl,
|
||||
id: href.value,
|
||||
originalEv: ev,
|
||||
},
|
||||
});
|
||||
env.bus.trigger("SCROLLER:ANCHOR_LINK_CLICKED", triggerEv);
|
||||
if (matchingEl && !triggerEv.defaultPrevented) {
|
||||
ev.preventDefault();
|
||||
scrollTo(matchingEl, { isAnchor: true });
|
||||
}
|
||||
|
||||
@@ -14,33 +14,35 @@ export function scrollTo(element, options = { scrollable: null, isAnchor: false
|
||||
return null;
|
||||
}
|
||||
|
||||
if (node.scrollHeight > node.clientHeight) {
|
||||
if (node.scrollHeight > node.clientHeight && node.clientHeight > 0) {
|
||||
return node;
|
||||
} else {
|
||||
return _getScrollParent(node.parentNode);
|
||||
}
|
||||
}
|
||||
const scrollable = options.scrollable ? options.scrollable : _getScrollParent(element);
|
||||
|
||||
// Scrollbar is present ?
|
||||
if (scrollable.scrollHeight > scrollable.clientHeight) {
|
||||
const scrollable = options.scrollable
|
||||
? options.scrollable
|
||||
: _getScrollParent(element.parentNode);
|
||||
if (scrollable) {
|
||||
const scrollBottom = scrollable.getBoundingClientRect().bottom;
|
||||
const scrollTop = scrollable.getBoundingClientRect().top;
|
||||
const elementBottom = element.getBoundingClientRect().bottom;
|
||||
const elementTop = element.getBoundingClientRect().top;
|
||||
if (elementBottom > scrollBottom) {
|
||||
// Scroll down
|
||||
if (options.isAnchor) {
|
||||
// For an anchor, the scroll place the element at the top
|
||||
scrollable.scrollTop += elementTop - scrollBottom + scrollable.clientHeight;
|
||||
} else {
|
||||
// The scroll make the element visible in the scrollable
|
||||
scrollable.scrollTop +=
|
||||
elementTop - scrollBottom + element.getBoundingClientRect().height;
|
||||
}
|
||||
} else if (elementTop < scrollTop) {
|
||||
// Scroll up
|
||||
if (elementBottom > scrollBottom && !options.isAnchor) {
|
||||
// The scroll place the element at the bottom border of the scrollable
|
||||
scrollable.scrollTop +=
|
||||
elementTop - scrollBottom + element.getBoundingClientRect().height;
|
||||
} else if (elementTop < scrollTop || options.isAnchor) {
|
||||
// The scroll place the element at the top of the scrollable
|
||||
scrollable.scrollTop -= scrollTop - elementTop;
|
||||
if (options.isAnchor) {
|
||||
// If the scrollable is within a scrollable, another scroll should be done
|
||||
const parentScrollable = _getScrollParent(scrollable.parentNode);
|
||||
if (parentScrollable) {
|
||||
scrollTo(scrollable, { isAnchor: true, scrollable: parentScrollable });
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ var viewUtils = require('web.viewUtils');
|
||||
var _t = core._t;
|
||||
var qweb = core.qweb;
|
||||
|
||||
const scrollUtils = require("@web/core/utils/scrolling");
|
||||
// symbol used as key to set the <field> node id on its widget
|
||||
const symbol = Symbol('form');
|
||||
|
||||
@@ -63,6 +64,20 @@ var FormRenderer = BasicRenderer.extend({
|
||||
this._applyFormSizeClass();
|
||||
return this._super.apply(this, arguments);
|
||||
},
|
||||
/**
|
||||
* Called each time the form view is attached into the DOM
|
||||
*/
|
||||
on_attach_callback: function () {
|
||||
this._super.apply(this, arguments);
|
||||
core.bus.on("SCROLLER:ANCHOR_LINK_CLICKED", this, this._onAnchorLinkClicked);
|
||||
},
|
||||
/**
|
||||
* Called each time the form view is detached into the DOM
|
||||
*/
|
||||
on_detach_callback: function () {
|
||||
this._super.apply(this, arguments);
|
||||
core.bus.off("SCROLLER:ANCHOR_LINK_CLICKED", this, this._onAnchorLinkClicked);
|
||||
},
|
||||
|
||||
//--------------------------------------------------------------------------
|
||||
// Public
|
||||
@@ -1219,6 +1234,47 @@ var FormRenderer = BasicRenderer.extend({
|
||||
var index = this.allFieldWidgets[this.state.id].indexOf(ev.data.target);
|
||||
this._activateNextFieldWidget(this.state, index);
|
||||
},
|
||||
/**
|
||||
* Ensure that the pane containing an anchor `element` that has been
|
||||
* targeted by a link will be visible in a notebook.
|
||||
* @param {CustomEvent} ev
|
||||
* @param {object} ev[detail] payload containing the element and the id to look for
|
||||
*
|
||||
*/
|
||||
_onAnchorLinkClicked(ev) {
|
||||
// Todo: we might need to search for the element elsewhere to know wich tab to activate
|
||||
// Maybe a t-if rule is hidding the target from the DOM
|
||||
const anchor = ev.detail.element || null;
|
||||
if (!anchor) {
|
||||
return;
|
||||
}
|
||||
function _getNotebookParent() {
|
||||
const notebook = anchor.closest(".o_notebook");
|
||||
// If the notebook is containing the element, return the notebook
|
||||
return notebook && notebook.contains(anchor) ? notebook : null;
|
||||
}
|
||||
function _setNotebookPage() {
|
||||
// Simulate a click on the nav link corresponding to the target pane
|
||||
const parentPane = anchor.closest(".tab-pane");
|
||||
if (notebook.contains(parentPane) && !parentPane.classList.contains(".active")) {
|
||||
const navLink = [...notebook.querySelectorAll(".nav-link")].filter((e) =>
|
||||
e.href.includes(parentPane.id)
|
||||
);
|
||||
navLink[0].click();
|
||||
}
|
||||
}
|
||||
|
||||
const notebook = _getNotebookParent();
|
||||
|
||||
// If the element is contained in a notebook, the page must be visible
|
||||
if (notebook) {
|
||||
_setNotebookPage();
|
||||
// Prevent the scroll to be handled by the scroller service itself
|
||||
ev.preventDefault();
|
||||
ev.detail.originalEv.preventDefault();
|
||||
scrollUtils.scrollTo(ev.detail.element, { isAnchor: true });
|
||||
}
|
||||
},
|
||||
/**
|
||||
* @private
|
||||
* @param {MouseEvent} ev
|
||||
|
||||
@@ -176,6 +176,10 @@ export function mapLegacyEnvToWowlEnv(legacyEnv, wowlEnv) {
|
||||
legacyEnv.bus.trigger("web_client_ready");
|
||||
});
|
||||
|
||||
wowlEnv.bus.on("SCROLLER:ANCHOR_LINK_CLICKED", null, (payload) => {
|
||||
legacyEnv.bus.trigger("SCROLLER:ANCHOR_LINK_CLICKED", payload);
|
||||
});
|
||||
|
||||
legacyEnv.bus.on("clear_cache", null, () => {
|
||||
wowlEnv.bus.trigger("CLEAR-CACHES");
|
||||
});
|
||||
|
||||
@@ -187,6 +187,139 @@ QUnit.test("Rendering with multiple anchors and scrolls", async (assert) => {
|
||||
assert.ok(isVisible(scrollableParent.querySelector("#anchor3")));
|
||||
});
|
||||
|
||||
QUnit.test("clicking anchor when no scrollable", async (assert) => {
|
||||
assert.expect(3);
|
||||
const scrollableParent = document.createElement("div");
|
||||
scrollableParent.style.overflow = "auto";
|
||||
scrollableParent.style.height = "150px";
|
||||
scrollableParent.style.width = "400px";
|
||||
target.append(scrollableParent);
|
||||
|
||||
class MyComponent extends Component {}
|
||||
MyComponent.template = tags.xml/* xml */ `
|
||||
<div class="o_content">
|
||||
<a href="#scrollToHere" class="btn btn-primary">scroll to ...</a>
|
||||
<div class="active-container">
|
||||
<p>There is no scrollable with only the height of this element</p>
|
||||
</div>
|
||||
<div class="inactive-container" style="max-height: 0">
|
||||
<h2>There should be no scrollable if this element has 0 height</h2>
|
||||
<p>
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue. Nulla tincidunt tincidunt mi. Curabitur iaculis, lorem vel rhoncus faucibus,
|
||||
felis magna fermentum augue, et ultricies lacus lorem varius purus. Curabitur eu amet.
|
||||
</p>
|
||||
<div id="scrollToHere">should try to scroll here only if scrollable!</div>
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
comp = await mount(MyComponent, { env, target: scrollableParent });
|
||||
assert.strictEqual(scrollableParent.scrollTop, 0);
|
||||
await click(scrollableParent, ".btn.btn-primary");
|
||||
assert.ok(scrollableParent.scrollTop === 0, "no scroll happened");
|
||||
scrollableParent.querySelector(".inactive-container").style.maxHeight = "unset";
|
||||
await click(scrollableParent, ".btn.btn-primary");
|
||||
assert.ok(scrollableParent.scrollTop !== 0, "a scroll happened");
|
||||
});
|
||||
|
||||
QUnit.test("clicking anchor when multi levels scrollables", async (assert) => {
|
||||
assert.expect(4);
|
||||
const scrollableParent = document.createElement("div");
|
||||
scrollableParent.style.overflow = "auto";
|
||||
scrollableParent.style.height = "150px";
|
||||
scrollableParent.style.width = "400px";
|
||||
target.append(scrollableParent);
|
||||
|
||||
class MyComponent extends Component {}
|
||||
MyComponent.template = tags.xml/* xml */ `
|
||||
<div class="o_content scrollable-1">
|
||||
<a href="#scroll1" class="btn1 btn btn-primary">go to level 2 anchor</a>
|
||||
<div>
|
||||
<p>This is some content</p>
|
||||
<p>
|
||||
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed non risus.
|
||||
Suspendisse lectus tortor, dignissim sit amet, adipiscing nec, ultricies sed,
|
||||
dolor. Cras elementum ultrices diam. Maecenas
|
||||
</p>
|
||||
</div>
|
||||
<div class="scrollable-2" style="background: green; overflow: auto; height: 100px;">
|
||||
<h2>This is level 1 of scrollable</h2>
|
||||
<p>
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue. Nulla tincidunt tincidunt mi. Curabitur iaculis, lorem vel rhoncus faucibus,
|
||||
felis magna fermentum augue, et ultricies lacus lorem varius purus. Curabitur eu amet.
|
||||
</p>
|
||||
<div style="background: lime;">
|
||||
<h2>This is level 2 of scrollable</h2>
|
||||
<p>
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue. Nulla tincidunt tincidunt mi. Curabitur iaculis, lorem vel rhoncus faucibus,
|
||||
felis magna fermentum augue, et ultricies lacus lorem varius purus. Curabitur eu amet.
|
||||
</p>
|
||||
<div id="scroll1" style="background: orange;">this element is contained in a scrollable metaverse!</div>
|
||||
<a href="#scroll2" class="btn2 btn btn-primary">go to level 1 anchor</a>
|
||||
<p>
|
||||
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed non risus.
|
||||
Suspendisse lectus tortor, dignissim sit amet, adipiscing nec, ultricies sed,
|
||||
dolor. Cras elementum ultrices diam. Maecenas ligula massa, varius a, semper
|
||||
congue, euismod non, mi. Proin porttitor, orci nec nonummy molestie, enim est
|
||||
eleifend mi, non fermentum diam nisl sit amet erat. Duis semper. Duis arcu
|
||||
massa, scelerisque vitae, consequat in, pretium a, enim. Pellentesque congue. Ut
|
||||
in risus volutpat libero pharetra tempor. Cras vestibulum bibendum augue. Praesent
|
||||
egestas leo in pede. Praesent blandit odio eu enim.
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div id="scroll2" style="background: orange;">this is an anchor at level 1!</div>
|
||||
<p>
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue. Nulla tincidunt tincidunt mi. Curabitur iaculis, lorem vel rhoncus faucibus,
|
||||
felis magna fermentum augue, et ultricies lacus lorem varius purus. Curabitur eu amet.
|
||||
</p>
|
||||
</div>
|
||||
`;
|
||||
comp = await mount(MyComponent, { env, target: scrollableParent });
|
||||
|
||||
const border = (el) => {
|
||||
// Returns the state of the element in relation to the borders
|
||||
const element = el.getBoundingClientRect();
|
||||
const scrollable = scrollableParent.getBoundingClientRect();
|
||||
return {
|
||||
top: parseInt(element.top - scrollable.top) < 10,
|
||||
bottom: parseInt(scrollable.bottom - element.bottom) < 10,
|
||||
};
|
||||
};
|
||||
|
||||
assert.strictEqual(scrollableParent.scrollTop, 0);
|
||||
await click(scrollableParent, ".btn1");
|
||||
assert.ok(
|
||||
border(scrollableParent.querySelector("#scroll1")).top,
|
||||
"the element must be near the top border"
|
||||
);
|
||||
assert.ok(
|
||||
border(scrollableParent.querySelector("#scroll1")).top,
|
||||
"the scrollable inside level 1 must be near the top border"
|
||||
);
|
||||
await click(scrollableParent, ".btn2");
|
||||
assert.ok(
|
||||
border(scrollableParent.querySelector("#scroll2")).top,
|
||||
"the element must be near the top border"
|
||||
);
|
||||
});
|
||||
|
||||
QUnit.test("Simple scroll to HTML elements", async (assert) => {
|
||||
assert.expect(6);
|
||||
const scrollableParent = document.createElement("div");
|
||||
|
||||
@@ -23,12 +23,19 @@ var _t = core._t;
|
||||
var createView = testUtils.createView;
|
||||
|
||||
const { registerCleanup } = require("@web/../tests/helpers/cleanup");
|
||||
const { legacyExtraNextTick, patchWithCleanup } = require("@web/../tests/helpers/utils");
|
||||
const { getFixture, legacyExtraNextTick, patchWithCleanup } = require("@web/../tests/helpers/utils");
|
||||
const { createWebClient, doAction } = require('@web/../tests/webclient/helpers');
|
||||
const { makeTestEnv } = require("@web/../tests/helpers/mock_env");
|
||||
const makeTestEnvironment = require("web.test_env");
|
||||
const { mapLegacyEnvToWowlEnv } = require("@web/legacy/utils");
|
||||
const { registry } = require("@web/core/registry");
|
||||
const { scrollerService } = require("@web/core/scroller_service");
|
||||
|
||||
let serverData;
|
||||
QUnit.module('Views', {
|
||||
beforeEach: function () {
|
||||
beforeEach: async function () {
|
||||
registry.category("services").add("scroller", scrollerService);
|
||||
|
||||
this.data = {
|
||||
partner: {
|
||||
fields: {
|
||||
@@ -1009,6 +1016,117 @@ QUnit.module('Views', {
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test("notebook page is changing when an anchor is clicked from another page", async (assert) => {
|
||||
assert.expect(6);
|
||||
|
||||
// This should be removed as soon as the view is moved to owl
|
||||
const wowlEnv = await makeTestEnv();
|
||||
const legacyEnv = makeTestEnvironment({ bus: core.bus });
|
||||
mapLegacyEnvToWowlEnv(legacyEnv, wowlEnv);
|
||||
|
||||
const scrollableParent = document.createElement("div");
|
||||
scrollableParent.style.overflow = "auto";
|
||||
const target = getFixture();
|
||||
target.append(scrollableParent);
|
||||
|
||||
var form = await createView({
|
||||
View: FormView,
|
||||
model: "partner",
|
||||
data: {
|
||||
partner: {
|
||||
fields: {},
|
||||
records: [
|
||||
{
|
||||
id: 1,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
arch: `<form string="Partners">
|
||||
<sheet>
|
||||
<notebook>
|
||||
<page string="Non scrollable page">
|
||||
<div id="anchor1">No scrollbar!</div>
|
||||
<a href="#anchor2" class="link2">TO ANCHOR 2</a>
|
||||
</page>
|
||||
<page string="Other scrollable page">
|
||||
<p style="font-size: large">
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue.
|
||||
</p>
|
||||
<p style="font-size: large">
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue.
|
||||
</p>
|
||||
<h2 id="anchor2">There is a scroll bar</h2>
|
||||
<a href="#anchor1" class="link1">TO ANCHOR 1</a>
|
||||
<p style="font-size: large">
|
||||
Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis,
|
||||
ligula massa adipiscing nisl, ac euismod nibh nisl eu lectus. Fusce vulputate sem
|
||||
at sapien. Vivamus leo. Aliquam euismod libero eu enim. Nulla nec felis sed leo
|
||||
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
|
||||
augue.
|
||||
</p>
|
||||
</page>
|
||||
</notebook>
|
||||
</sheet>
|
||||
</form>`,
|
||||
res_id: 1,
|
||||
});
|
||||
scrollableParent.append(form.el);
|
||||
|
||||
// We set the height of the parent to the height of the second pane
|
||||
// We are then sure there will be no scrollable on this pane but a
|
||||
// only for the first pane
|
||||
scrollableParent.style.maxHeight =
|
||||
scrollableParent.querySelector(".o_action").getBoundingClientRect().height + "px";
|
||||
|
||||
// The element must be contained in the scrollable parent (top and bottom)
|
||||
const isVisible = (el) => {
|
||||
return (
|
||||
el.getBoundingClientRect().bottom <= scrollableParent.getBoundingClientRect().bottom &&
|
||||
el.getBoundingClientRect().top >= scrollableParent.getBoundingClientRect().top
|
||||
);
|
||||
};
|
||||
|
||||
assert.ok(
|
||||
scrollableParent
|
||||
.querySelector(".tab-pane.active")
|
||||
.contains(scrollableParent.querySelector("#anchor1")),
|
||||
"the first pane is visible"
|
||||
);
|
||||
assert.ok(
|
||||
!isVisible(scrollableParent.querySelector("#anchor2")),
|
||||
"the second anchor is not visible"
|
||||
);
|
||||
scrollableParent.querySelector(".link2").click();
|
||||
assert.ok(
|
||||
scrollableParent
|
||||
.querySelector(".tab-pane.active")
|
||||
.contains(scrollableParent.querySelector("#anchor2")),
|
||||
"the second pane is visible"
|
||||
);
|
||||
assert.ok(
|
||||
isVisible(scrollableParent.querySelector("#anchor2")),
|
||||
"the second anchor is visible"
|
||||
);
|
||||
scrollableParent.querySelector(".link1").click();
|
||||
assert.ok(
|
||||
scrollableParent
|
||||
.querySelector(".tab-pane.active")
|
||||
.contains(scrollableParent.querySelector("#anchor1")),
|
||||
"the first pane is visible"
|
||||
);
|
||||
assert.ok(isVisible(scrollableParent.querySelector("#anchor1")), "the first anchor is visible");
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('invisible attrs on group are re-evaluated on field change', async function (assert) {
|
||||
assert.expect(2);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user