Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix misplaced cursor on LB/LR paging on specific conditions #811

Merged
merged 1 commit into from Mar 2, 2023

Conversation

Gemba
Copy link

@Gemba Gemba commented Sep 25, 2022

@Gemba Gemba force-pushed the fix_misplaced_cursor_on_lb_lr_paging branch from eba8d14 to ae3952b Compare November 3, 2022 07:07
@Gemba
Copy link
Author

Gemba commented Nov 3, 2022

Corrected position of visible list area after launch of game from screensaver.

Including fix for cursor highlighting in visible list area when game is launched from screensaver.

Squashed into one commit.
@Gemba Gemba force-pushed the fix_misplaced_cursor_on_lb_lr_paging branch from 8661402 to 33fdbb2 Compare February 18, 2023 13:04
@pjft
Copy link
Collaborator

pjft commented Feb 24, 2023

Apologies, yet another PR that arrived during my summer break and that completely flew under the radar.

@cmitu you end up looking into this specific topic, just to check?

@cmitu
Copy link

cmitu commented Feb 24, 2023

@pjft I think I have briefly looked at this, but to be honest I don't remember what was the result.

@Gemba
Copy link
Author

Gemba commented Feb 27, 2023

As a refresher, also to myself, this PR addresses the testcases (see also test list in referenced forum, http://ix.io/48K3) which fail (marked N). With the PR all testcases should pass. I did test the full list and it also did not create any regression.

I can elaborate on the testcase list if it is too brief or puzzles you.

@pjft
Copy link
Collaborator

pjft commented Feb 28, 2023

Thanks. I appreciate the detail of the tests and checking for regressions there. This is a moderately opaque area of the code and I don't know enough about it to make any relevant observations here, so if the tests are ok that's all that matters.

Question: did you confirm if/how this behaves when we have an empty collection and with filters? Just to confirm that nothing breaks there.

If all is good, I'll be happy to squash it.

Thanks.

@Gemba
Copy link
Author

Gemba commented Mar 2, 2023

Good point. To be safe, I tested the filter settings:

  1. When a list is reduced and contains at least one entry, the cursor will be reset to initial position, the same as when navigating into a system after ES boot. Navigation is as expected.
  2. When the applied filter(s) result in an empty list the Text "No Entries Found" is displayed and no cursor move is possible. As expected.
  3. When the filter(s) are removed the cursor position is as expected as in 1.

Did I miss a filter scenario to test?

@pjft pjft merged commit 4389185 into RetroPie:master Mar 2, 2023
@pjft
Copy link
Collaborator

pjft commented Mar 2, 2023

I think you covered it all then. Thank you!

@Gemba Gemba deleted the fix_misplaced_cursor_on_lb_lr_paging branch February 3, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants