ScrollTo API on List widget

[AnkushJadhav]

Description:

Exposed a ScrollTo API that scrolls a List widget and sets the focus on the ListItem represented by the id given to the API.

Fixes #2105

Checklist:

• Tests included.

• Lint and formatter run with no errors.

• Tests all pass.

• Public APIs match existing style.

[Jacalz]

Nick work. Just a little suggestion to simplify a math operation.

[Jacalz]

Thanks. This looks good to me 👍

Just one little note. This seems to incorrectly mention fixing #1892 which is specific to the table widget. Did you plan to add in a fix for that as well?

[andydotxyz]

An api “ScrollTo” should not change selection, there is a separate API that does that already.

Also the initial issue requests ScrollToBottom, would that and ScrollToTop be useful too?

[AnkushJadhav]

Thanks. This looks good to me 👍

Just one little note. This seems to incorrectly mention fixing #1892 which is specific to the table widget. Did you plan to add in a fix for that as well?

@Jacalz No, I think I added that one incorrectly. Not sure what I was thinking. Updated the PR description.

[AnkushJadhav]

An api “ScrollTo” should not change selection, there is a separate API that does that already.

Also the initial issue requests ScrollToBottom, would that and ScrollToTop be useful too?

@andydotxyz The "ScrollTo" API does not change the selection. It simply scrolls to the item provided to it. Any selection made on the list remains intact.
A ScrollToTop and ScrollToBottom seemed more like utility functions to me, that don't add much value. The developer can easily implement these functions themselves by using the ScrollTo API. Your thoughts?

[AnkushJadhav]

@andydotxyz The existing Select API modified the selection and also scrolled to the selection. While the new ScrollTo API ONLY scrolls to the item. Since both these APIs have a "scroll to item" kind of logic in them, so extracted that logic into a common helper function.

[andydotxyz]

I understand now - apologies that I misread the code.
I was confused by the use of the word "Focus" when you were just meaning scroll to make visible. Can you please update the method name so it does not imply focus change.

[andydotxyz]

@andydotxyz The existing Select API modified the selection and also scrolled to the selection. While the new ScrollTo API ONLY scrolls to the item. Since both these APIs have a "scroll to item" kind of logic in them, so extracted that logic into a common helper function.

You're quite right, apologies. The method name tripped me up, can you find a better name?

[andydotxyz]

A ScrollToTop and ScrollToBottom seemed more like utility functions to me, that don't add much value

I was just suggesting it because those were added to container.Scroll at the request of previous users.

[AnkushJadhav]

A ScrollToTop and ScrollToBottom seemed more like utility functions to me, that don't add much value

I was just suggesting it because those were added to container.Scroll at the request of previous users.

Oh, then it makes sense to have them for consistency. Updated the PR.

[andydotxyz]

I think there is an off-by-one in there.

[andydotxyz]

Great, thanks

[andydotxyz]

It seems that GitHub has a policy or blocking first-time contributor CI. I have asked it to progress now.
As a result there are some staticcheck errors to fix, sorry!

[andydotxyz]

I am so sorry for forgetting about this (my head was in bug fixing) but each of the new methods needs the following comment added:

//
// Since: 2.1

[AnkushJadhav]

@andydotxyz Thanks for pointing that out. Sorry, this is my first time on a big open source project, so still learning all the nitty-gritty.

[andydotxyz]

@andydotxyz Thanks for pointing that out. Sorry, this is my first time on a big open source project, so still learning all the nitty-gritty.

Don't apologise, we have a lot of processes to keep the API need and consistent. There is a load written down at https://github.com/fyne-io/fyne/wiki/Contributing