Skip to content

Commit 382ab9d

Browse files
[Workspace Chrome] Adjust scrollIntoViewIfNeeded for the new layout (#228796)
## Summary We're working on [new layout engine](elastic/kibana-team#1581) for Kibana moving from legacy fixed layout to grid layout. New layout is in main behind feature flag `feature_flags.overrides.core.chrome.layoutType: 'grid'` and we're preparing for QA before the rollout. When running functional tests with the new layout there turned out to be a lot of failures that I pin pointed to `src/platform/packages/shared/kbn-ftr-common-functional-ui-services/services/web_element_wrapper/scroll_into_view_if_necessary.js` This function is used for scrolling inside our ftr test. It didn't work properly with the new layout because the main application scroll is moved from document to internal main grid cell. This PR adjusts the function to work well for both the old layout and the new layout. However, there were issues with this function in the old layout as well. It didn't correctly decide if an element was visible when the page was scrolled down. I fixed it, but I had to adjust a couple of tests to account for the logical change. @dolaru helped to run mki, and it passed: https://buildkite.com/elastic/appex-qa-serverless-kibana-ftr-tests/builds/6385, give a bit more confidence --------- Co-authored-by: kibanamachine <[email protected]>
1 parent f879c02 commit 382ab9d

File tree

11 files changed

+116
-122
lines changed

11 files changed

+116
-122
lines changed

NOTICE.txt

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -138,32 +138,6 @@ The above copyright notice and this permission notice shall be included in all c
138138

139139
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
140140

141-
---
142-
Based on the scroll-into-view-if-necessary module from npm
143-
https://github.com/stipsan/compute-scroll-into-view/blob/master/src/index.ts#L269-L340
144-
145-
MIT License
146-
147-
Copyright (c) 2018 Cody Olsen
148-
149-
Permission is hereby granted, free of charge, to any person obtaining a copy
150-
of this software and associated documentation files (the "Software"), to deal
151-
in the Software without restriction, including without limitation the rights
152-
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
153-
copies of the Software, and to permit persons to whom the Software is
154-
furnished to do so, subject to the following conditions:
155-
156-
The above copyright notice and this permission notice shall be included in all
157-
copies or substantial portions of the Software.
158-
159-
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
160-
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
161-
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
162-
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
163-
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
164-
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
165-
SOFTWARE.
166-
167141
---
168142
MIT License
169143

src/platform/packages/shared/kbn-ftr-common-functional-ui-services/services/find.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,14 +415,14 @@ export class FindService extends FtrService {
415415
public async clickByCssSelector(
416416
selector: string,
417417
timeout: number = this.defaultFindTimeout,
418-
topOffset?: number
418+
topOffsetOrOptions?: number | { topOffset?: number; bottomOffset?: number }
419419
): Promise<void> {
420420
this.log.debug(`Find.clickByCssSelector('${selector}') with timeout=${timeout}`);
421421
await this.retry.try(async () => {
422422
const element = await this.byCssSelector(selector, timeout);
423423
if (element) {
424424
// await element.moveMouseTo();
425-
await element.click(topOffset);
425+
await element.click(topOffsetOrOptions);
426426
} else {
427427
throw new Error(`Element with css='${selector}' is not found`);
428428
}

src/platform/packages/shared/kbn-ftr-common-functional-ui-services/services/test_subjects.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,14 @@ export class TestSubjects extends FtrService {
163163
public async click(
164164
selector: string,
165165
timeout: number = this.FIND_TIME,
166-
topOffset?: number
166+
topOffsetOrOptions?: number | { topOffset?: number; bottomOffset?: number }
167167
): Promise<void> {
168168
this.log.debug(`TestSubjects.click(${selector})`);
169-
await this.findService.clickByCssSelector(testSubjSelector(selector), timeout, topOffset);
169+
await this.findService.clickByCssSelector(
170+
testSubjSelector(selector),
171+
timeout,
172+
topOffsetOrOptions
173+
);
170174
}
171175

172176
public async pressEnter(selector: string, timeout: number = this.FIND_TIME): Promise<void> {

src/platform/packages/shared/kbn-ftr-common-functional-ui-services/services/web_element_wrapper/scroll_into_view_if_necessary.js

Lines changed: 0 additions & 73 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
export function scrollIntoViewIfNecessary(
11+
target: HTMLElement,
12+
scrollContainerId?: string,
13+
scrollFixedHeader: number = 0,
14+
scrollFixedFooter: number = 0
15+
) {
16+
const scrollContainer =
17+
(scrollContainerId && document.getElementById(scrollContainerId)) || document.documentElement;
18+
19+
// Measure helper, logic is different for scrolling the viewport vs a scroller element
20+
const getRects = () => {
21+
const targetRect = target.getBoundingClientRect();
22+
const scrollIsViewport =
23+
scrollContainer === document.documentElement || scrollContainer === document.body;
24+
const scrollRect = scrollIsViewport
25+
? { top: 0, left: 0, right: window.innerWidth, bottom: window.innerHeight }
26+
: scrollContainer.getBoundingClientRect();
27+
return { targetRect, scrollRect };
28+
};
29+
30+
const isVisible = ({ targetRect, scrollRect }: ReturnType<typeof getRects>) =>
31+
targetRect.top >= scrollRect.top + scrollFixedHeader &&
32+
targetRect.bottom <= scrollRect.bottom - scrollFixedFooter &&
33+
targetRect.left >= scrollRect.left &&
34+
targetRect.right <= scrollRect.right;
35+
36+
let { targetRect, scrollRect } = getRects();
37+
38+
if (isVisible({ targetRect, scrollRect })) {
39+
return;
40+
}
41+
42+
// Determine scroll container for adjustments
43+
44+
// First try native scrollIntoView - it will find and scroll the correct container
45+
target.scrollIntoView();
46+
47+
if (scrollFixedHeader) {
48+
// Remeasure after initial scroll
49+
({ targetRect, scrollRect } = getRects());
50+
51+
// Adjust for fixed headers
52+
const deltaTop = targetRect.top - (scrollRect.top + scrollFixedHeader);
53+
if (deltaTop < 0) {
54+
scrollContainer.scrollBy({ top: deltaTop });
55+
}
56+
}
57+
58+
if (scrollFixedFooter) {
59+
// Remeasure again after header adjustment
60+
({ targetRect, scrollRect } = getRects());
61+
62+
// Adjust for fixed footers
63+
const deltaBottom = targetRect.bottom - (scrollRect.bottom - scrollFixedFooter);
64+
if (deltaBottom > 0) {
65+
scrollContainer.scrollBy({ top: deltaBottom });
66+
}
67+
}
68+
}

src/platform/packages/shared/kbn-ftr-common-functional-ui-services/services/web_element_wrapper/web_element_wrapper.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import { PNG } from 'pngjs';
1414
import cheerio from 'cheerio';
1515
import { subj as testSubjSelector } from '@kbn/test-subj-selector';
1616
import type { ToolingLog } from '@kbn/tooling-log';
17+
import { APP_MAIN_SCROLL_CONTAINER_ID } from '@kbn/core-chrome-layout-constants';
1718
import type { CustomCheerio, CustomCheerioStatic } from './custom_cheerio_api';
18-
// @ts-ignore not supported yet
19+
1920
import { scrollIntoViewIfNecessary } from './scroll_into_view_if_necessary';
2021
import { Browsers } from '../remote/browsers';
2122

@@ -199,9 +200,9 @@ export class WebElementWrapper {
199200
*
200201
* @return {Promise<void>}
201202
*/
202-
public async click(topOffset?: number) {
203+
public async click(topOffsetOrOptions?: number | { topOffset?: number; bottomOffset?: number }) {
203204
await this.retryCall(async function click(wrapper) {
204-
await wrapper.scrollIntoViewIfNecessary(topOffset);
205+
await wrapper.scrollIntoViewIfNecessary(topOffsetOrOptions);
205206
await wrapper._webElement.click();
206207
});
207208
}
@@ -780,6 +781,7 @@ export class WebElementWrapper {
780781
await this.driver.executeScript(
781782
scrollIntoViewIfNecessary,
782783
this._webElement,
784+
APP_MAIN_SCROLL_CONTAINER_ID,
783785
topOffset || this.fixedHeaderHeight,
784786
bottomOffset
785787
);

src/platform/packages/shared/kbn-ftr-common-functional-ui-services/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,6 @@
1414
"@kbn/ftr-common-functional-services",
1515
"@kbn/std",
1616
"@kbn/expect",
17+
"@kbn/core-chrome-layout-constants",
1718
]
1819
}

src/platform/test/functional/page_objects/dashboard_page_controls.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class DashboardPageControls extends FtrService {
4848
private readonly retry = this.ctx.getService('retry');
4949
private readonly browser = this.ctx.getService('browser');
5050
private readonly testSubjects = this.ctx.getService('testSubjects');
51+
private readonly panelActions = this.ctx.getService('dashboardPanelActions');
5152

5253
private readonly common = this.ctx.getPageObject('common');
5354

@@ -402,7 +403,11 @@ export class DashboardPageControls extends FtrService {
402403
public async optionsListOpenPopover(controlId: string) {
403404
this.log.debug(`Opening popover for Options List: ${controlId}`);
404405
await this.retry.try(async () => {
405-
await this.testSubjects.click(`optionsList-control-${controlId}`);
406+
await this.testSubjects.click(
407+
`optionsList-control-${controlId}`,
408+
500,
409+
await this.panelActions.getContainerTopOffset()
410+
);
406411
await this.retry.waitForWithTimeout('popover to open', 500, async () => {
407412
return await this.testSubjects.exists(`optionsList-control-popover`);
408413
});

src/platform/test/functional/services/dashboard/panel_actions.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
import type { WebElementWrapper } from '@kbn/ftr-common-functional-ui-services';
11+
import { asyncMap } from '@kbn/std';
1112
import { FtrService } from '../../ftr_provider_context';
1213

1314
const ACTION_SHOW_CONFIG_PANEL_SUBJ = 'embeddablePanelAction-ACTION_SHOW_CONFIG_PANEL';
@@ -32,7 +33,6 @@ export class DashboardPanelActionsService extends FtrService {
3233
private readonly find = this.ctx.getService('find');
3334
private readonly inspector = this.ctx.getService('inspector');
3435
private readonly testSubjects = this.ctx.getService('testSubjects');
35-
private readonly browser = this.ctx.getService('browser');
3636

3737
private readonly header = this.ctx.getPageObject('header');
3838
private readonly common = this.ctx.getPageObject('common');
@@ -47,10 +47,26 @@ export class DashboardPanelActionsService extends FtrService {
4747
}
4848

4949
async getDashboardContainerTopOffset() {
50-
return (
51-
(await (await this.testSubjects.find('dashboardContainer')).getPosition()).y +
52-
DASHBOARD_MARGIN_SIZE
53-
);
50+
const fixedHeaders = (
51+
await Promise.all([
52+
// global fixed eui headers, TODO: remove when Kibana switched to grid layout
53+
this.find.allByCssSelector('[data-fixed-header="true"]', 500),
54+
// fixed header with actions from project navigation
55+
this.find.allByCssSelector('[data-test-subj="kibanaProjectHeaderActionMenu"]', 500),
56+
// sticky unified search bar
57+
this.find.allByCssSelector('[data-test-subj="globalQueryBar"]', 500),
58+
])
59+
).flat();
60+
61+
let fixedHeaderHeight = 0;
62+
await asyncMap(fixedHeaders, async (header) => {
63+
const headerHeight = (await header.getSize()).height;
64+
fixedHeaderHeight += headerHeight;
65+
});
66+
67+
const additionalOffsetForFloatingHoverActions = 32;
68+
69+
return fixedHeaderHeight + DASHBOARD_MARGIN_SIZE + additionalOffsetForFloatingHoverActions;
5470
}
5571

5672
async getCanvasContainerTopOffset() {
@@ -70,17 +86,10 @@ export class DashboardPanelActionsService extends FtrService {
7086
async scrollPanelIntoView(wrapper?: WebElementWrapper) {
7187
this.log.debug(`scrollPanelIntoView`);
7288
wrapper = wrapper || (await this.getPanelWrapper());
73-
const yOffset = (await wrapper.getPosition()).y;
74-
await this.browser.execute(`
75-
const scrollY = window.scrollY;
76-
window.scrollBy(0, scrollY - ${yOffset});
77-
`);
7889

7990
const containerTop = await this.getContainerTopOffset();
8091

81-
await wrapper.moveMouseTo({
82-
topOffset: containerTop,
83-
});
92+
await wrapper.moveMouseTo({ topOffset: containerTop });
8493
}
8594

8695
async toggleContextMenu(wrapper?: WebElementWrapper) {

x-pack/solutions/security/test/cloud_security_posture_functional/page_objects/add_cis_integration_form_page.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,9 @@ export function AddCisIntegrationFormPageProvider({
346346
if (credentialType === 'cloud_connectors') {
347347
credentialTypeValue = 'cloud_connector';
348348
}
349-
await testSubjects.click(AWS_CREDENTIAL_SELECTOR);
349+
await testSubjects.click(AWS_CREDENTIAL_SELECTOR, undefined, {
350+
bottomOffset: 100 /* account for fixed footer to decide if need to scroll down */,
351+
});
350352
await selectValue(AWS_CREDENTIAL_SELECTOR, credentialTypeValue);
351353
};
352354

0 commit comments

Comments
 (0)