BrowserViewportScroller not honoring native scroll margin

[mmorris8]

Which @angular/* package(s) are the source of the bug?

router

Is this a regression?

No

Description

Angular ignores the scroll margin property - particularly scroll-margin-top, instead always scrolling the target to 0,0. The native HTML anchor scrolling honors this property, so this behavior must be considered a bug.

Please provide a link to a minimal reproduction of the bug

This is a userland solution to the bug, at least for scrollMarginTop (which is the most common use case)

export class AppComponent {
  constructor(
    private router: Router,
    private viewportScroller: ViewportScroller,
  ) {
    router.events.pipe(filter((event: Event): event is Scroll => event instanceof Scroll)
    ).subscribe(e => {
      const element = document.querySelector(`#${e.anchor}`)

      if (element) {
        const offset = parseFloat(getComputedStyle(element).scrollMarginTop)
        viewportScroller.setOffset([0, offset])
      }
    });
  }
}

Please provide the exception or error you saw

No error message is emitted in the console.

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 17.0.7
Node: 21.7.1 (Unsupported)
Package Manager: npm 10.5.0
OS: darwin arm64

Angular: 17.0.7
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.7
@angular-devkit/build-angular   17.0.7
@angular-devkit/core            17.0.7
@angular-devkit/schematics      17.0.7
@angular/material               17.0.4
@schematics/angular             17.0.7
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else?

No response

[atscott]

This looks more like a BrowserViewportScroller issue rather than a Router one. scrollToElement relies only on the internal offset and does not look at the styles on the element. There's no runnable reproduction in this report but I suspect if you provided a custom implementation that does that instead of what's here, it would work correctly.

[mmorris8]

That would be the function that needs to change for this bug to be fixed.

And I call it out as a bug because intuitively the behavior of this scrollTo should match the native behavior of hashes. The scrollMargin css property exists precisely because of the sticky and fixed headers. The offset property can remain for backwards compatibility, but the properties need to be honored because that's why they exist. They are honored by all browsers except IE.

The below should work. I'll look into figuring out how to do a pull request over the weekend.

private scrollToElement(el: HTMLElement): void {
    const rect = el.getBoundingClientRect();
    const left = rect.left + this.window.pageXOffset;
    const top = rect.top + this.window.pageYOffset;
    const marginLeft = parseFloat(getComputedStyle(el).scrollMarginLeft);
    const marginTop = parseFloat(getComputedStyle(el).scrollMarginTop);

    const offset = this.offset();
    this.window.scrollTo(left + marginLeft - offset[0], top + marginTop - offset[1]);
  }

[atscott]

Right, I agree.

They are honored by all browsers except IE

This, historically, would be the reason the issue exists. The implementation here has existed since before IE was dropped and potentially before the current set of browsers all fully supported it as well.

[michael-lloyd-morris]

This is the account that I will be issuing the PR from. I'll look up the guidelines and try to get something in on Saturday. I imagine test revision will be the largest segment.