scrollOptions configuration is not working as expected

[erwanlfrt]

ISSUE : scrollOptions configuration is not working as expected

A. The issue

According to the documentation of vue-js-spatial-navigation about scrollOptions :

The page will not scroll to the focus element when setting scrollOptions to "" or null.

However, in _spatialnavigation.js, focusNScroll function, l. 605 :

var focusNscroll = function () {
  if (_sections[sectionId].scrollOptions !== undefined && _sections[sectionId].scrollOptions !== "") {
    elem.focus({ preventScroll: true });
    elem.scrollIntoView(_sections[sectionId].scrollOptions);
  } else if (GlobalConfig.scrollOptions !== undefined && GlobalConfig.scrollOptions !== "") {
    elem.focus({ preventScroll: true });
    elem.scrollIntoView(GlobalConfig.scrollOptions);
  } else {
    elem.focus();
  }
};

The value of scrollOptions is not compared to null so the element.scrollIntoView will be executed into the first "if condition".

B. Solutions to fix it...

1. Considering scrollOptions as a global configuration AND a section configuration

If we compare scrollOptions into the "if" condition and the "else if" condition then the "else" will be executed. But there is no preventScroll into the "else" section so the page will scroll to the focus element contrary to the documentation.

So the best correction is :

var focusNscroll = function () {
  if (_sections[sectionId].scrollOptions !== undefined && _sections[sectionId].scrollOptions !== "" && _sections[sectionId].scrollOptions !== null) {
    elem.focus({ preventScroll: true });
    elem.scrollIntoView(_sections[sectionId].scrollOptions);
  } else if (GlobalConfig.scrollOptions !== undefined && GlobalConfig.scrollOptions !== "" && GlobalConfig.scrollOptions !== null) {
    elem.focus({ preventScroll: true });
    elem.scrollIntoView(GlobalConfig.scrollOptions);
  } else {
    elem.focus({ preventScroll: true });
  }
};

I'm afraid that the preventScroll into the "else" may change the behavior of existing app.

2. Considering scrollOptions exclusively as a global configuration

Still according to the vue-js-spatial-navigation, scrollOptions is classified as an Optional global Configuration.

Into index.js, assignConfig function, l. 15 :

const assignConfig = (sectionId, config) => {
      let sectionConfig = Object.assign({}, globalConfig);
      if (config) {
        Object.assign(sectionConfig, config);
      }
      ...
    };

Each section received the global config scrollOptions if not declared in the config of the section, which is a little bit weird for a global configuration. So if scrollOptions has to be considered exclusively as a global configuration, it changes the correction of focusNScroll. All we need is to check the global configuration :

var focusNscroll = function () {
  if (GlobalConfig.scrollOptions !== undefined && GlobalConfig.scrollOptions !== "" && GlobalConfig.scrollOptions !== null) {
    elem.focus({ preventScroll: true });
    elem.scrollIntoView(GlobalConfig.scrollOptions);
  } else {
    elem.focus({ preventScroll: true });
  }
};

However, it's interesting to have the possibility to set the scrollOptions for each section, in my opinion the first correction is the best and we can classified the scrollOptions both as a global configuration AND a section configuration into the README.md file.

Thanks for paying attention, Have a pleasant day.

[spacerefugee]

Agree with the first solution is the better one. I will patch it soon. Thanks for pointing it out.