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

Fixes shift click selection after programmatic selection in most cases, Issue #2469 #3213

Merged
merged 5 commits into from
Jun 1, 2024

Conversation

RRomeroJr
Copy link
Contributor

@RRomeroJr RRomeroJr commented May 28, 2024

Proposed fix for issue #2469
Original thread:
https://forums.ankiweb.net/t/bug-arrow-keys-in-preview-break-shift-click-selection-in-card-browser/29282

Some things I noticed while looking into this.

  • Changing the selection programmatically followed by shift-click selecting is what causes the issue.
    This would happen when:

    • < / > buttons were clicked in the preview window
    • Ctrl + N or Ctrl+ P table shortcuts were used in the browser window
    • Home or End table shortcuts were used in the browser window

    Then..

    • Shift + Left Click selecting in the table view
  • Some quick googling showed that this was a common issue with Qt, and I reproduced it in a small mockup program I made. I'm not sure if it is a bug with Qt itself, or if there is some better/ more proper way to select items in a TableView from code.

What I added feels like a work-around more than a proper fix, but, as far as I can tell, it fixes all the mentioned scenarios.

However, if the user holds Shift while clicking the < or > buttons in the preview window the same bug reappears. I haven't been able to figure out why this still happens.

@RRomeroJr RRomeroJr marked this pull request as ready for review May 28, 2024 06:02
@RRomeroJr
Copy link
Contributor Author

Sorry about making 2 pull requests. In hindsight I could've fixed the other one.

If this is acceptable, I'd like to ask one question. I'd like to try to make an automated test for this behavior, but I'm not sure were to start looking implement it. If possible, any advice on where to look would be appreciated.

@dae
Copy link
Member

dae commented May 28, 2024

Thanks for looking into this! Just one minor request: please link to the issue in the comment, to make it a little easier to find the related discussion.

What I added feels like a work-around more than a proper fix, but, as far as I can tell, it fixes all the mentioned scenarios.

Even if it's not perfect, if it's less bad than what we currently have, that's a win.

@RRomeroJr
Copy link
Contributor Author

RRomeroJr commented May 28, 2024

Added link up top 👍

@dae
Copy link
Member

dae commented Jun 1, 2024

Thank you!

@dae dae merged commit 81de397 into ankitects:main Jun 1, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants