pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: scrolling to anchor working badly

Browse Source 
This commit fixes a bug when you clicked on a link related to an anchor.
e.g. an <a> tag with an href="#anchor". The scroll to the element
with id="anchor" didn't happen because the position was not found correctly.
The fix uses getBoundingCLientRect to get more precise positions
of the element with the anchor and the scrollable area.
A test is introduced to check scrolling for multiple anchors in a template
with a more complex structure (Note: the test fails without the fix).
The scroll behaves differently depending if the target is an anchor or
an element. An other test has been added for the command palette about this
behavior.

closes odoo/odoo#79310

X-original-commit: b0da44d62f85d6530f8e568025f7fea25c5454c5
Signed-off-by: FrancoisGe <fge@odoo.com>
This commit is contained in:
luvi
2021-11-03 11:48:46 +00:00
parent 837e72fbfa
commit d304be57be
6 changed files with 333 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/core/commands/command_palette.js
+1 -1
View File
@@ -212,7 +212,7 @@ export class CommandPalette extends Component {
const listbox = this.el.querySelector(".o_command_palette_listbox");
const command = listbox.querySelector(`#o_command_${nextIndex}`);
scrollTo(command, listbox);
scrollTo(command, { scrollable: listbox });
}
onCommandClicked(index) {
addons/web/static/src/core/dropdown/dropdown_navigation_hook.js
+1 -1
View File
@@ -184,7 +184,7 @@ export function useDropdownNavigation() {
menuElement = menuElements[nextIndex];
}
menuElement.makeOnlyActive();
scrollTo(menuElement.el, menuElement.el.parentElement);
scrollTo(menuElement.el, { scrollable: menuElement.el.parentElement });
}
};
addons/web/static/src/core/scroller_service.js
+1 -1
View File
@@ -43,7 +43,7 @@ export const scrollerService = {
}
if (matchingEl) {
ev.preventDefault();
scrollTo(matchingEl);
scrollTo(matchingEl, { isAnchor: true });
}
}
}
addons/web/static/src/core/utils/scrolling.js
+19 -8
View File
@@ -4,9 +4,11 @@
* Ensures that `element` will be visible in its `scrollable`.
*
* @param {HTMLElement} element
* @param {HTMLElement} [scrollable]
* @param {Object} options
* @param {HTMLElement} options[scrollable] a scrollable area
* @param {Boolean} options[isAnchor] states if the scroll is to an anchor
*/
export function scrollTo(element, scrollable = null) {
export function scrollTo(element, options = { scrollable: null, isAnchor: false }) {
function _getScrollParent(node) {
if (node == null) {
return null;
@@ -18,18 +20,27 @@ export function scrollTo(element, scrollable = null) {
return _getScrollParent(node.parentNode);
}
}
scrollable = scrollable ? scrollable : _getScrollParent(element);
const scrollable = options.scrollable ? options.scrollable : _getScrollParent(element);
// Scrollbar is present ?
if (scrollable.scrollHeight > scrollable.clientHeight) {
const scrollBottom = scrollable.clientHeight + scrollable.scrollTop;
const elementBottom = element.offsetTop + element.offsetHeight;
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
scrollable.scrollTop = elementBottom - scrollable.clientHeight;
} else if (element.offsetTop < scrollable.scrollTop) {
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
scrollable.scrollTop = element.offsetTop;
scrollable.scrollTop -= scrollTop - elementTop;
}
}
}
addons/web/static/tests/core/commands/command_palette_tests.js
+158
View File
@@ -732,6 +732,164 @@ QUnit.test("press enter on command", async (assert) => {
assert.verifySteps(["C2"]);
});
QUnit.test("keyboard navigation scroll", async (assert) => {
testComponent = await mount(TestComponent, { env, target });
const commands = [
{ name: "Command1" },
{ name: "Command2" },
{ name: "Command3" },
{ name: "Command4" },
];
const providers = [
{
provide: () => commands,
},
];
const config = {
providers,
};
env.services.dialog.add(CommandPaletteDialog, {
config,
});
const isVisible = (el) => {
// Returns the visibility of the element in the scrollable element
return (
el.getBoundingClientRect().bottom <=
target.querySelector(".o_command_palette_listbox").getBoundingClientRect().bottom &&
el.getBoundingClientRect().top >=
target.querySelector(".o_command_palette_listbox").getBoundingClientRect().top
);
};
const border = (el) => {
// Returns the state of the element in relation to the borders
const element = el.getBoundingClientRect();
const scrollable = target
.querySelector(".o_command_palette_listbox")
.getBoundingClientRect();
return {
top: element.top === scrollable.top,
bottom: element.bottom === scrollable.bottom,
};
};
await nextTick();
// The listbox height is set to be lower than the list of commands
// to assure the command palette is scrollable. The palette is only able to
// display three rows of commands so we are sure we always have one row
// element out of bounds
target.querySelectorAll(".o_command").forEach((e) => (e.style.height = "50px"));
target.querySelector(".o_command_palette_listbox").style.maxHeight = "150px";
target.querySelector(".o_command_category").style.padding = "0";
assert.containsOnce(target, ".o_command_palette");
assert.containsN(target, ".o_command", 4);
let focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
!isVisible(target.querySelector("#o_command_3")),
"commands 1-2-3 are visible"
);
assert.ok(
border(focusedCommand).top && !border(focusedCommand).bottom,
"the focus is at the top border"
);
triggerHotkey("arrowdown");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
!isVisible(target.querySelector("#o_command_3")),
"commands 1-2-3 are visible"
);
assert.ok(isVisible(target.querySelector("#o_command_1")), "the second element is visible");
assert.ok(
!border(focusedCommand).top && !border(focusedCommand).bottom,
"the focus does not reach a border"
);
triggerHotkey("arrowdown");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
!isVisible(target.querySelector("#o_command_3")),
"commands 1-2-3 are visible"
);
assert.ok(
!border(focusedCommand).top && border(focusedCommand).bottom,
"the focus has reached the bottom border"
);
triggerHotkey("arrowdown");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
!isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
isVisible(target.querySelector("#o_command_3")),
"commands 2-3-4 are visible"
);
assert.ok(
!border(focusedCommand).top && border(focusedCommand).bottom,
"the focus is still at the bottom border"
);
triggerHotkey("arrowup");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
!isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
isVisible(target.querySelector("#o_command_3")),
"commands 2-3-4 are visible"
);
assert.ok(
!border(focusedCommand).top && !border(focusedCommand).bottom,
"the focus does not reach a border"
);
triggerHotkey("arrowup");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
!isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
isVisible(target.querySelector("#o_command_3")),
"commands 2-3-4 are visible"
);
assert.ok(
border(focusedCommand).top && !border(focusedCommand).bottom,
"the focus has reached the top border"
);
triggerHotkey("arrowup");
await nextTick();
focusedCommand = target.querySelector(".o_command.focused");
assert.ok(
isVisible(target.querySelector("#o_command_0")) &&
isVisible(target.querySelector("#o_command_1")) &&
isVisible(target.querySelector("#o_command_2")) &&
!isVisible(target.querySelector("#o_command_3")),
"commands 1-2-3 are visible"
);
assert.ok(
border(focusedCommand).top && !border(focusedCommand).bottom,
"the focus is still at the top border"
);
});
QUnit.test("multi level command", async (assert) => {
testComponent = await mount(TestComponent, { env, target });
const emptyMessageByNamespace = {
addons/web/static/tests/core/scroller_service_tests.js
+153
View File
@@ -2,6 +2,7 @@
import { registry } from "@web/core/registry";
import { scrollerService } from "@web/core/scroller_service";
import { scrollTo } from "@web/core/utils/scrolling";
import { registerCleanup } from "../helpers/cleanup";
import { makeTestEnv } from "../helpers/mock_env";
import { click, getFixture, nextTick } from "../helpers/utils";
@@ -111,3 +112,155 @@ QUnit.test("Simple rendering with a scroll", async (assert) => {
await click(scrollableParent, ".btn.btn-primary");
assert.ok(scrollableParent.scrollTop !== 0);
});
QUnit.test("Rendering with multiple anchors and scrolls", async (assert) => {
assert.expect(4);
const scrollableParent = document.createElement("div");
scrollableParent.style.overflow = "scroll";
scrollableParent.style.height = "150px";
scrollableParent.style.width = "400px";
target.append(scrollableParent);
class MyComponent extends Component {}
MyComponent.template = tags.xml/* xml */ `
<div class="o_content">
<h2 id="anchor3">ANCHOR 3</h2>
<a href="#anchor1" class="link1">sroll to ...</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>
<p>
Ut velit mauris, egestas sed, gravida nec, ornare ut, mi. Aenean ut orci vel massa
suscipit pulvinar. Nulla sollicitudin. Fusce varius, ligula non tempus aliquam, nunc
turpis ullamcorper nibh, in tempus sapien eros vitae ligula. Pellentesque rhoncus
nunc et augue. Integer id felis. Curabitur aliquet pellentesque diam. Integer quis
metus vitae elit lobortis egestas. Lorem ipsum dolor sit amet, consectetuer adipiscing
elit. Morbi vel erat non mauris convallis vehicula. Nulla et sapien. Integer tortor
tellus, aliquam faucibus, convallis id, congue eu, quam.
</p>
<table>
<tbody>
<tr>
<td>
<h2 id="anchor2">ANCHOR 2</h2>
<a href="#anchor3" class="link3">TO ANCHOR 3</a>
<p>
The table forces you to get the precise position of the element.
</p>
</td>
</tr>
</tbody>
</table>
<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.
</p>
<div id="anchor1">sroll here!</div>
<a href="#anchor2" class="link2">TO ANCHOR 2</a>
</div>
`;
comp = await mount(MyComponent, { env, target: scrollableParent });
assert.strictEqual(scrollableParent.scrollTop, 0);
await click(scrollableParent, ".link1");
// 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(isVisible(scrollableParent.querySelector("#anchor1")));
await click(scrollableParent, ".link2");
assert.ok(isVisible(scrollableParent.querySelector("#anchor2")));
await click(scrollableParent, ".link3");
assert.ok(isVisible(scrollableParent.querySelector("#anchor3")));
});
QUnit.test("Simple scroll to HTML elements", async (assert) => {
assert.expect(6);
const scrollableParent = document.createElement("div");
scrollableParent.style.overflow = "scroll";
scrollableParent.style.height = "150px";
scrollableParent.style.width = "400px";
target.append(scrollableParent);
class MyComponent extends Component {}
MyComponent.template = tags.xml/* xml */ `
<div class="o_content">
<p>
Aliquam convallis sollicitudin purus.
</p>
<div id="o-div-1">A div is an HTML element</div>
<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>
<p>
Ut velit mauris, egestas sed, gravida nec, ornare ut, mi. Aenean ut orci vel massa
suscipit pulvinar. Nulla sollicitudin. Fusce varius, ligula non tempus aliquam, nunc
turpis ullamcorper nibh, in tempus sapien eros vitae ligula. Pellentesque rhoncus
nunc et augue. Integer id felis. Curabitur aliquet pellentesque diam. Integer quis
metus vitae elit lobortis egestas. Lorem ipsum dolor sit amet, consectetuer adipiscing
elit. Morbi vel erat non mauris convallis vehicula. Nulla et sapien. Integer tortor
tellus, aliquam faucibus, convallis id, congue eu, quam.
</p>
<div id="o-div-2">A div is an HTML element</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
placerat imperdiet. Aenean suscipit nulla in justo. Suspendisse cursus rutrum
augue.
</p>
</div>
`;
comp = await mount(MyComponent, { env, target: scrollableParent });
assert.strictEqual(scrollableParent.scrollTop, 0);
// 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
);
};
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) === 0,
bottom: parseInt(scrollable.bottom - element.bottom) === 0,
};
};
// When using scrollTo to an element, this should just scroll
// until the element is visible in the scrollable parent
const div_1 = scrollableParent.querySelector("#o-div-1");
const div_2 = scrollableParent.querySelector("#o-div-2");
assert.ok(isVisible(div_1) && !isVisible(div_2), "only the first div is visible");
assert.ok(!border(div_1).top, "the element is not at the top border");
scrollTo(div_2);
assert.ok(!isVisible(div_1) && isVisible(div_2), "only the second div is visible");
assert.ok(border(div_2).bottom, "the element must be at the bottom border");
scrollTo(div_1);
assert.ok(border(div_1).top, "the element must be at the top border");
});