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

Clean up mod list spacebar handling in GUI #2543

Merged
merged 1 commit into from Oct 23, 2018

Conversation

HebaruSan
Copy link
Member

Problem

If you focus the mod list and press space in GUI, some surprising behaviors can occur depending on exactly where you click:

animation

  • If you click the Install checkbox, then the checkbox will toggle when you press space and then again when you let go
  • If you click the Upgrade checkbox, then pressing space can toggle both checkboxes

Causes

CKAN has some special code to toggle checkboxes when you press space.

  • The grid itself handles checkbox toggling in KeyDown, but we were doing it in KeyPress, which happens later
  • Our code fired regardless of which cell was focused, and if that cell had a checkbox, then multiple toggles would happen at different times
  • Our code only ever toggled the Install checkbox, so if you click an upgradable row and press space, it would try to remove the mod rather than upgrade it

Changes

Now the behavior is more consistent and less surprising.

  • Now we handle space in KeyDown for consistency with the built in behavior
  • If one of the checkbox cells has the focus, we let the grid handle it
  • If there's an Update checkbox, we toggle it preferentially, otherwise we toggle the Install checkbox

Fixes #2534.

@HebaruSan HebaruSan added Bug GUI Issues affecting the interactive GUI Pull request labels Oct 21, 2018
@politas politas merged commit d3e6edd into KSP-CKAN:master Oct 23, 2018
politas added a commit that referenced this pull request Oct 23, 2018
@HebaruSan HebaruSan deleted the fix/modlist-spacebar branch October 23, 2018 14:52
@7ranceaddic7
Copy link

Speaking of surprising behaviour, I stumbled an edge case where spacebar crashes CKAN.

Steps to Replicate:
Launch CKAN.
Select desires instance.
DO NOTHING - Don't click anything. Don't Arrow or Tab anywhere. NOTHING.
Hit Spacebar.
CTD

Does this fix handle that behavior? If not, I can open new issue.

@HebaruSan
Copy link
Member Author

@7ranceaddic7, go ahead and submit a new issue. We can close after investigating it if it seems to be already fixed. Be sure to specify the steps clearly, for example in the above it's not clear whether you're pressing space on the list of instances or the list of mods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug GUI Issues affecting the interactive GUI Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants