JavaScript scroll behaviour conflicts with scroll-padding-top

[martindholmes]

After retrieving search results, we use:

window.scroll({ top: this.resultsDiv.offsetTop, behavior: "smooth" });

to navigate past the search controls to the actual results. However, if there is a fixed header, it will obscure the first part of the result div.

One workaround for this might be to try to retrieve the appropriate scroll-padding-top value and reduce the scroll value accordingly:

window.scroll({ top: document.getElementById('ssResults').offsetTop - parseInt(document.body.style.scrollPaddingTop), behavior: "smooth" });

This requires that the site designer has set a scroll-padding-top on the body, but presumably they should have done that if they're using a fixed header.

[martindholmes]

This fix should be applied in both dev and the release branch.

[joeytakeda]

I don't think that solution will work generally, though, since if the scrollPaddingTop hasn't been declared, the value would be NaN. So a few ideas:

If we wanted to handle this in a default way, we could just catch the NaN value and make it 0:

const offsetTop = document.getElementById('ssResults').offsetTop
const scrollPaddingTop = document.body.style.scrollPaddingTop || 0;
window.scroll({top: offsetTop - scrollPaddingTop, behavior: "smooth"})

Another, perhaps more flexible way we could do this, though, is splitting the scrolling call (which is currently add the end of doSearch) into its own method, which could then be overridden simply:

StaticSearch.prototype.scrollToResults = () => {
        window.scroll({ 
             top: document.getElementById('ssResults').offsetTop - parseInt(document.body.style.scrollPaddingTop), 
             behavior: "smooth"
        })
}

This would also have the advantage of making it straightforward to stop automatic scrolling (i.e. by passing a function that does nothing) or have some sort of custom scrolling thing if they'd like

Alternatively, if we didn't want to split it out, we could just make the window.scroll options configurable in the constructor:

this.scrollToOptions = {
             top: document.getElementById('ssResults').offsetTop, 
             behavior: "smooth"
}

Which could then be overridden similarly of course:

Sch.scrollToOptions.top = document.getElementById('ssResults').offsetTop - parseInt(document.body.style.scrollPaddingTop)

//....
window.scroll(this.scrollToOptions)

[martindholmes]

I think I prefer the separate method for scrolling; sometimes scrolling is not required at all, as for example when the search controls are in a sidebar.

[martindholmes]

This turned out to be way more tricky than I thought, since the scroll-padding and scroll-margin properties can be set on all sorts of different page components. A better option turned out to be simply this:

  scrollToResults(){
    try{
      this.resultsDiv.scrollIntoView({behavior: 'smooth', block: 'center'});
    }
    catch(e){
      return false;
    }
  }

This brings the results div to the centre of the page, which should be enough to avoid any encroaching fixed or sticky components, without having to try to calculate anything; it's now a method so that it's easily overridable. PR coming for dev. In 1.4 branch, I'll just tweak the single call directly.

[martindholmes]

After some more testing, I switched to using block: 'start'; with long result-sets, the beginning of the result-set ends up offscreen using center. I'm leaving this open because I haven't yet made the change in the dev branch; that's a TODO, and I'd like to tweak the test page layout in dev to make it a true test of this optimization.

[martindholmes]

I've deleted the previous branch and removed the PR because I'm still not really happy with how this is working. For version 2.0, I'd like to have a fully-realized solution the always brings the top of the search results into view for the user, whatever the configuration of sticky or fixed components on the page.

[martindholmes]

I've now created a new branch where I've abstracted the scroll call to an overridable method. This is the only sane approach, as far as I can see, because there's no way to know what fixed-position elements might appear in any particular context, so there's no generic solution to ensuring that the top of the results display div is in view. Anyone with a project that uses a fixed header can easily override the method to allow for the fixed elements they know they have in their project.

[martindholmes]

PR #307.

[martindholmes]

Done.