Desktop: Fix Goto scrolling (#2122)

[rc2dev]

Fix issue #2122

[tessus]

The fix works. I've tested it on macOS. I'm just wondering why the removed code was there in the first place.

@laurent22 do you remember why that code was there?

I can only say that removing those 2 lines fixes the problem.

[laurent22]

The original code makes sense to me: if the note is already visible, there's no need to scroll. So I don't think we can remove this part but rather we should find out why a valid index if outside of bounds. @rccavalcanti, do you know what could be the issue?

[rc2dev]

I dove a bit more on that line and found it just has a problem in the boundaries:

• If the 1st visible item is selected, it doesn't return (it should).
• If the 1st invisible item is selected, it does return (it shouldn't). This caused Goto anything doesn't scroll as expected #2122.

So I brought the line back and fixed it.

Edit: Clarification.

[tessus]

Tested it on macOS. Your fix works! 👍

[rc2dev]

Great! Just don't merge yet. I'm working in a better fix.

[rc2dev]

So previously, I found that bottom and top were 1-based, while itemIndex was 0-based. This ultimately caused the boundary problem. I solved it in 62674dc, resting 1 from them.

However, I thought this was probably unintended and all values should be 0-based. So I went to investigate further.

It turns out that I found 2 different bugs in the class that caused the 1-based index:

• for bottom, it was in the definition of bottomItemIndex
• for top, it was in its own definition

So I fixed both and kept the line of conditional return unchanged.

I think this is much cleaner and probably the final solution. Sorry for the multiple iterations!

[tessus]

@laurent22 is anything still missing?

[laurent22]

Perfect thanks @rccavalcanti and @tessus!