Prevent Scroll to First Selection on Select/Unselect

[MicahBrown]

Resolves issue for multiselects using closeOnSelect: false that caused the list of results to scroll to the first selection after each select/unselect

This pull request includes a

• Bug fix
• New feature
• Translation

The following changes were made

• Removed behavior for scrolling to first selection after each selection/unselection

If this is related to an existing ticket, include a link to it as well.
#1513

[lvmajor]

Just a suggestion, instead of commenting out the feature for the highlightFirstItem prototype, maybe you could simply comment the line where it highlights the first item on select (line 277 in your version of the code now)?
This way when closing/reopening the selectbox, you get the first item in sight, which IMO should be the case. What is your opinion about that?

[MicahBrown]

@os1r1s110 I revised it.

[alexweissman]

@MicahBrown is there any indication in the commit history as to why self.highlightFirstItem(); was added in the first place? I want to be sure that removing these lines doesn't cause a regression in other features. See #4417 for an alternate proposal.

A test or two to assert the behavior that this patch fixes, would also be appreciated.

[MicahBrown]

@alexweissman Unfortunately, I don't know if I'll have time to write any tests since I'm very busy now. I did do a quick look-through on the commit history and found that the self.highlightFirstItem() was first introduced in this commit: e897d00 (it's been renamed from self.focusFirstItem()). When it was first introduced, it didn't have the this.ensureHighlightVisible() call, which is what's causing the issue now and bringing the first item into view. I don't see why we would want self.highlightFirstItem() called on the select or unselect triggers, so that's why I took it out of those triggers and left the results:all one. Hopefully that helps!

Also, we've been using my change in production for a few months now and haven't noticed any regressions. I know that doesn't cover all cases, but thought I'd mention it.

[alexweissman]

No problem, and thanks for getting back to me so quickly.

It would appear that highlightFirstItem was added for the infinite scroll (pagination) feature, as per @jgbishop's comment:

I'd argue that the "highlight first item" feature should be adjusted to work either only in the infinite scroll case (while the popup is open and the user is scrolling), or via a user-set option.

[alexweissman]

Please see #4417 (comment) to understand why we cannot simply remove the self.highlightFirstItem() calls entirely. We need to add some logic so that self.highlightFirstItem() is only called when infinite scroll (pagination) is used.

[alexweissman]

I notice that #4625 also adds some conditional logic for scrolling to the first item, to handle some other use cases.

It almost seems like maybe "scroll-after-select" should be a configurable option?

[alexweissman]

Please continue the discussion for this feature in #5150.