Refactor/chat messages initial scroll#2105
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new @siemens/map-styles library, integrating it into the existing maps-ng package and updating build, release, and TypeScript configurations accordingly. Additionally, the SiChatContainerComponent is refactored to use afterNextRender instead of ngAfterContentInit for scrolling to the bottom. The reviewer recommends simplifying this scrolling logic by removing the redundant nested requestAnimationFrame calls inside afterNextRender to prevent potential layout flashes and unnecessary delays.
| afterNextRender(() => { | ||
| // Double rAF: first flush lets layout/style settle (including ResizeObserver first fire), | ||
| // second runs after the browser has painted — at that point scrollHeight is final. | ||
| requestAnimationFrame(() => { | ||
| requestAnimationFrame(() => { | ||
| const container = this.messagesContainer(); | ||
| if (container && !this.noAutoScroll()) { | ||
| const el = container.nativeElement; | ||
| el.scrollTop = el.scrollHeight; | ||
| } | ||
| this.initialScrollDone = true; | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Using nested requestAnimationFrame calls inside afterNextRender is redundant and can cause a visible layout flash (flicker) for the user.
Angular's afterNextRender is guaranteed to run after the next paint cycle has completed and the DOM has been fully rendered. Therefore, the layout and style have already settled, and scrollHeight is already final. Delaying the scroll by two additional frames (~33ms) is unnecessary and results in a delayed scroll to the bottom.
We can simplify this by performing the scroll directly inside afterNextRender.
afterNextRender(() => {
const container = this.messagesContainer();
if (container && !this.noAutoScroll()) {
const el = container.nativeElement;
el.scrollTop = el.scrollHeight;
}
this.initialScrollDone = true;
});
spliffone
force-pushed
the
refactor/chat-messages-initial-scroll
branch
from
4724e24 to
d084af2
Compare
spliffone
force-pushed
the
refactor/chat-messages-initial-scroll
branch
from
d084af2 to
17897ec
Compare
spliffone
force-pushed
the
refactor/chat-messages-initial-scroll
branch
from
d8b14bd to
bb1b659
Compare
…ll deterministically Switch to the earlyRead/write phases of afterNextRender so the scrollHeight read forces a layout commit before the scrollTop write. Wire ResizeObserver and MutationObserver up inside the same write phase to prevent observer callbacks from racing the initial scroll. Re-pin scroll position after document.fonts.ready resolves so font swaps after first paint no longer shift the captured viewport in VRT snapshots.
…ypography These snapshots were re-recorded locally and drift by 1-2 px against the canonical mcr.microsoft.com/playwright:v1.59.1-noble runner image. Restore the main baselines and let the vrt-update workflow regenerate them on the CI image if needed.
…cal CI image Generated via the vrt-update workflow run 26600901940 on the mcr.microsoft.com/playwright:v1.59.1-noble image. Replaces the previous baseline that drifted by 2 px on Linux font rasterization.
…ner output The vrt-update workflow generated a 1671px tall baseline, but the actual e2e runs (same Docker image) consistently produce 1669px. Restore main's canonical baseline so the dark snapshot test passes again.
…tion-delete The 'should create remove criteria' test flakily failed in vrt-update environments because focus after removing the Event pill is not guaranteed to remain on the free-text search input. The next Backspace was therefore not always routed to the input, so deleting the Location criterion never triggered and the assertion timed out. Restore the same explicit focus pattern used elsewhere in the file before pressing Backspace to delete the Location criterion.
Refresh three snapshots that the vrt-update workflow regenerated to match the current Linux CI runner output: - si-page-header small (dark) - si-markdown-renderer (dark) - typography type-styles (light)
1 participant
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: