impr(commandline): scroll active entry into view when searchbar is empty (JayTailor45)

[JayTailor45]

Description

It improves language selection dropdown by adding scroll into view. Now selected values will be automatically scrolled into view.

Closes #4662

#4662

[Miodec]

Please remove the animation and only animate if the $("#commandLine input").val() is empty, otherwise scroll to top.

[JayTailor45]

@Miodec I have updated the code. Now my function looks like this. Can you please let me know best way to test both cases?

function scrollSelectedLanguageToView(): void {
  const suggestions = $(".suggestions");
  const scrollTarget = $(".suggestions .entry .icon i.fa-check");
  if (suggestions.length && scrollTarget.length) {
    const paddingOffset = 8;
    const suggestionsOffset = suggestions.offset();
    const scrollTargetOffset = scrollTarget.offset();
    if (suggestionsOffset && scrollTargetOffset) {
      if (!$("#commandLine input").val()) {
        $(".suggestions").animate(
          {
            scrollTop:
              scrollTargetOffset.top - suggestionsOffset.top - paddingOffset,
          },
          500
        );
      } else {
        $(".suggestions").scrollTop(
          scrollTargetOffset.top - suggestionsOffset.top - paddingOffset
        );
      }
    }
  }
}

[Miodec]

The animation is still there and the element is still scrolled to the top instead of the center.

[Miodec]

The results are also not scrolled to the top.

[Miodec]

Steps to test with:

Set language to russian.
Open the commandline, go to the language selection.
The results should snap to russian being in the middle. (currently russian is begin snapped to the top)
As soon as i start typing, for example wanting to switch to spanish, the results should snap to the top (currently after pressing s, they are still scrolled down)

[JayTailor45]

@Miodec I've made changes as you mentioned. Please check it and let me know if any changes require. Thanks!

Steps to test with:

Set language to russian. Open the commandline, go to the language selection. The results should snap to russian being in the middle. (currently russian is begin snapped to the top) As soon as i start typing, for example wanting to switch to spanish, the results should snap to the top (currently after pressing s, they are still scrolled down)

[Miodec]

Another issue is that if you scroll the list to the currently active element, the selected element is still the first one, making it a bit confusing.

[JayTailor45]

Oh I thought it was intentional. No worries. I will make the change ;)

Another issue is that if you scroll the list to the currently active element, the selected element is still the first one, making it a bit confusing.

[JayTailor45]

@Miodec I've added the active element logic as you asked. Please check it and let me know. also let me know if you need squashed commit. Thanks!

Another issue is that if you scroll the list to the currently active element, the selected element is still the first one, making it a bit confusing.

[Miodec]

It looks like this .suggestions .entry .icon i.fa-check element check is used both here and in updateSuggested. I think instead of always scrolling the checked element into view, we convert this function to be a more generic scrollActiveEntryToCenter() which scrolls an entry to center which has an index of activeIndex (global variable in this file)

Also, i would move the if ($("#commandLine input").val()) check to the updateSuggested() funciton aswell.

[Miodec]

I just realised there is a function for that already - keepActiveEntryInView(). I think this whole function is not needed.

[JayTailor45]

@Miodec thanks for the feedback. I have refactored the code to use the existing keepActiveEntryInView().

[Miodec]

Well done, thanks.

[JayTailor45]

My pleasure! Thank you for the opportunity.