[Workspace Chrome] Adjust scrollIntoViewIfNeeded for the new layout #228796
[Workspace Chrome] Adjust scrollIntoViewIfNeeded for the new layout #228796Dosant merged 16 commits intoelastic:mainfrom
scrollIntoViewIfNeeded for the new layout #228796Conversation
64331ac to
9660677
Compare
9660677 to
45d6339
Compare
scrollIntoViewIfNeeded to work with new layout
scrollIntoViewIfNeeded to work with new layout scrollIntoViewIfNeeded for the new layout
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
nreese
left a comment
There was a problem hiding this comment.
kibana-presentation changes LGTM
code review only
|
|
||
| let fixedHeaderHeight = 0; | ||
| await Promise.all( | ||
| fixedHeaders.map(async (header) => { |
There was a problem hiding this comment.
How about using import { asyncMap } from '@kbn/std'; instead of fixedHeaders.map wrapped in Promise.all?
There was a problem hiding this comment.
it seems that
await Promise.all(
fixedHeaders.map(async (header) => {
const headerHeight = (await header.getSize()).height;
fixedHeaderHeight += headerHeight;
})
);
becomes
await asyncMap(fixedHeaders, async (header) => {
const headerHeight = (await header.getSize()).height;
fixedHeaderHeight += headerHeight;
});
I am not sure it adds much to readability? and we don't need the limit.
But I'll keep it since I had to try it anyway.
Unless I am not missing more ways to simplify
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#9146[✅] x-pack/platform/test/functional/apps/lens/group3/config.ts: 10/10 tests passed. |
|
Random flaky test run looks good . We agreed with @dolaru and @dmlemeshko to merge after next serverless promotion to give more time for fixes and reverts if we see instabilities |
…x-custom-scorll-into-view
…:Dosant/kibana into d/2025-07-21-fix-custom-scorll-into-view
…:Dosant/kibana into d/2025-07-21-fix-custom-scorll-into-view
nreese
left a comment
There was a problem hiding this comment.
kibana-presentation changes LGTM
code review only
maxcold
left a comment
There was a problem hiding this comment.
cloud_security_posture test changes LGTM
|
Will merge after on-week |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Unknown metric groupsESLint disabled in files
Total ESLint disabled count
History
|
Summary
We're working on new layout engine 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.jsThis 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