Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run scroll command via Angular afterRender hook #1182

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −27
Open

Run scroll command via Angular afterRender hook #1182

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 39 additions & 27 deletions mesop/web/src/shell/shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
HostListener,
NgZone,
Renderer2,
afterRender,
} from '@angular/core';
import {Router, RouterOutlet, Routes, provideRouter} from '@angular/router';
import {MatProgressBarModule} from '@angular/material/progress-bar';
Expand Down Expand Up @@ -62,6 +63,8 @@ export class Shell {
rootComponent!: ComponentProto;
private resizeSubject = new Subject<void>();

private commandScrollKey = '';

constructor(
private zone: NgZone,
private renderer: Renderer2,
Expand All @@ -81,6 +84,10 @@ export class Shell {
this.resizeSubject
.pipe(debounceTime(500))
.subscribe(() => this.onResizeDebounced());

afterRender(() => {
this.maybeExecuteScrollCommand();
});
}

ngOnInit() {
Expand Down Expand Up @@ -116,33 +123,10 @@ export class Shell {
this.channel.resetOverridedTitle();
}
} else if (command.hasScrollIntoView()) {
// Scroll into view
const key = command.getScrollIntoView()!.getKey();
// Schedule scroll into view to run after the current event loop tick
// so that the component has time to render.
setTimeout(() => {
const targetElements = document.querySelectorAll(
`[data-key="${key}"]`,
);
if (!targetElements.length) {
console.error(
`Could not scroll to component with key ${key} because no component found`,
);
return;
}
if (targetElements.length > 1) {
console.warn(
'Found multiple components',
targetElements,
'to potentially scroll to for key',
key,
'. This is probably a bug and you should use a unique key identifier.',
);
}
targetElements[0].parentElement!.scrollIntoView({
behavior: 'smooth',
});
}, 0);
// Store the scroll key so we can defer execution of scroll command until
// after everything is fully rendered. This helps avoid race conditions
// with the scroll behavior.
this.commandScrollKey = command.getScrollIntoView()!.getKey() || '';
} else if (command.hasSetPageTitle()) {
this.channel.setOverridedTitle(
command.getSetPageTitle()!.getTitle() || '',
Expand Down Expand Up @@ -257,6 +241,34 @@ export class Shell {
userEvent.setResize(new ResizeEvent());
this.channel.dispatch(userEvent);
}

// Executes the scroll command if a key has been specified.
maybeExecuteScrollCommand() {
if (this.commandScrollKey) {
const targetElements = document.querySelectorAll(
`[data-key="${this.commandScrollKey}"]`,
);
if (!targetElements.length) {
console.error(
`Could not scroll to component with key ${this.commandScrollKey} because no component found`,
);
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably would be good to do this before the return:
this.commandScrollKey = '';

this branch is already an edge case, but having the command scroll key somehow run in a future render loop when the the target element exists, seems like an even more bizzare edge case.

}
if (targetElements.length > 1) {
console.warn(
'Found multiple components',
targetElements,
'to potentially scroll to for key',
this.commandScrollKey,
'. This is probably a bug and you should use a unique key identifier.',
);
}
targetElements[0].parentElement!.scrollIntoView({
behavior: 'smooth',
});
this.commandScrollKey = '';
}
}
}

const routes: Routes = [{path: '**', component: Shell}];
Expand Down
Loading