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 home/end, url handler, typing nav, and dependency double click #2727

Merged
merged 1 commit into from
Apr 20, 2019

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 13, 2019

Problem

If you do any of the following:

  • Press Home on the mod list
  • Press End on the mod list
  • Invoke the URL handler
  • Start typing a mod name on the mod list
  • Double click a mod in the mod info dependency tree

... this exception occurs:

System.InvalidOperationException: Die aktuelle Zelle kann nicht auf eine unsichtbare Zelle festgelegt werden. (Rough translation: the current cell can't be set to an invisible cell)
   bei System.Windows.Forms.DataGridView.set_CurrentCell(DataGridViewCell value)
   bei CKAN.Main.ModList_KeyDown(Object sender, KeyEventArgs e)
   bei System.Windows.Forms.Control.OnKeyDown(KeyEventArgs e)
   bei System.Windows.Forms.DataGridView.OnKeyDown(KeyEventArgs e)
   bei System.Windows.Forms.Control.ProcessKeyEventArgs(Message& m)
   bei System.Windows.Forms.DataGridView.ProcessKeyEventArgs(Message& m)
   bei System.Windows.Forms.Control.WmKeyChar(Message& m)
   bei System.Windows.Forms.Control.WndProc(Message& m)
   bei System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

Cause

These all work by setting ModList.CurrentCell to a row's third cell, for reasons explained here:

CKAN/GUI/Main.cs

Lines 1055 to 1056 in 3bc5376

// Setting this to the 'Name' cell prevents the checkbox from being toggled
// by pressing 'Space' while the row is not indicated as active.

However, as of #2671, the third cell is Replace instead of Name, and as of #2690, it's hidden most of the time. A hidden cell cannot be the current cell, so an exception is thrown. This is essentially the same problem as #2703, but it's not specific to the mark-all-updates operation.

Changes

Now we look for usable columns in this order:

  1. The current column, if any, to avoid scrolling horizontally
  2. The first visible non-checkbox column, if any
  3. The Installed checkbox column (always visible fallback)

In addition, the fix for #2704 is modified to use this same logic. I think the original reasoning from the comment quoted above applies here, and so we should avoid setting focus to checkboxes when we can.

Fixes KSP-CKAN/NetKAN#7108.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request labels Apr 13, 2019
@DasSkelett DasSkelett removed the Bug Something is not working as intended label Apr 13, 2019
@HebaruSan HebaruSan added the Bug Something is not working as intended label Apr 13, 2019
@DasSkelett
Copy link
Member

Why does it show my name for the label remove (and in other occasions)?
Is this because I am the last one who logged in on the Waffel page?

@HebaruSan
Copy link
Member Author

Yeah, that Waffle thing likes to make automatic changes for reasons I don't understand. It did it in my name for a while when I first joined as well.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine, no errors, no unwanted horizontal scrolling or something.

@HebaruSan
Copy link
Member Author

Welcome to the team by the way, @DasSkelett! I'm excited about having a more... active... collaborator. :)

@DasSkelett
Copy link
Member

Thanks. You tell me something, that was quite a hot start with you going on vacation, and @politas releasing a new major version, and all that exactly in those two weeks where I had training every two days for a fire brigade performance test....

@HebaruSan HebaruSan merged commit 3b553bb into KSP-CKAN:master Apr 20, 2019
HebaruSan added a commit that referenced this pull request Apr 20, 2019
@Olympic1 Olympic1 removed Bug Something is not working as intended Pull request labels Apr 20, 2019
@HebaruSan HebaruSan deleted the fix/home-end-crash branch April 21, 2019 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hitting 'home' / 'Pos1' key on modlist throws error.
3 participants