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

Add right-click context menu #2202

Merged
merged 10 commits into from
Dec 28, 2017
Merged

Conversation

Olympic1
Copy link
Member

@Olympic1 Olympic1 commented Nov 30, 2017

As said in #2180, here is a right click context menu. I already added a Download Contents and a Reinstall option.

The Reinstall option is just a simple uninstall -> install process. I'll work on that a bit more to show the process in the WaitTabPage and maybe a possibility to uninstall the mod whitout uninstalling its dependants.

I also added a check for the left mouse button to double clicking.

Possible implementations:

@Olympic1 Olympic1 added Enhancement New features or functionality GUI Issues affecting the interactive GUI In progress We're still working on this Pull request ★★☆ labels Nov 30, 2017
@Olympic1 Olympic1 added this to the 1.24.0 milestone Nov 30, 2017
@Olympic1 Olympic1 self-assigned this Nov 30, 2017
@Olympic1 Olympic1 added In progress We're still working on this ★★☆ and removed In progress We're still working on this ★★☆ labels Nov 30, 2017
@HebaruSan
Copy link
Member

Can Reinstall work for a mod that the user manually uninstalled? That's a large percentage of issue reports that come in, so it would be very helpful if we could simply say "right click ModX and choose Reinstall".

@Olympic1
Copy link
Member Author

I haven't actually tested that, but I need to work on reinstall anyway. But now time to go to bed.

Copy link
Member

@politas politas left a comment

Choose a reason for hiding this comment

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

This is great work, and it makes me soooo happy! if we can just exclude autodetected mods for this PR, I'd love to merge it.

GUI/Main.cs Outdated

downloadContentsToolStripMenuItem.Enabled = !guiMod.IsCached;

if (guiMod.IsInstalled || guiMod.IsAutodetected)
Copy link
Member

Choose a reason for hiding this comment

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

Properly handling AD mods is a whole can of worms, that I think would be better handled in a separate PR. There's a lot of different cases to consider. As this is, this throws a ModNotInstalledKraken for AD mods in ModuleInstaller.UninstallList that isn't being caught in the transaction below and thus crashes the client. Can we just make this if ( guiMod.IsInstalled )? Then I can merge this and we can all work on extending the right-click functionality one menu item at a time.

Copy link
Member

@politas politas left a comment

Choose a reason for hiding this comment

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

If you could make that one change, this should be fine to merge as a solid basis for future functionality. Converting AD mods to CKAN control would be better as a plugin, I think, thus maintaining our promise that CKAN won't touch files it didn't install.

@Olympic1
Copy link
Member Author

Sorry for waiting, I had to do 12h weeks. Will make the changes

GUI/Main.cs Outdated
@@ -453,7 +427,7 @@ public void UpdateCKAN()
SwitchEnabledState();
ClearLog();
tabController.RenameTab("WaitTabPage", "Updating CKAN");
SetDescription("Upgrading CKAN to " + AutoUpdate.Instance.LatestVersion);
SetDescription("Upgrading CKAN to {AutoUpdate.Instance.LatestVersion}");
Copy link
Member

@HebaruSan HebaruSan Dec 26, 2017

Choose a reason for hiding this comment

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

Missing the dollar sign before the starting quote, I believe this will print the literal curly braces as text.

Copy link
Member Author

Choose a reason for hiding this comment

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

How did VS screw that up?

GUI/Main.cs Outdated
Repo.default_ckan_repo.ToString()
);
(
Path.Combine(CurrentInstance.GameDir(), "CKAN/GUIConfig.xml"),
Copy link
Member

Choose a reason for hiding this comment

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

Let's use CurrentInstance.CkanDir().

@Olympic1
Copy link
Member Author

PR should be good to go now.

@politas politas merged commit aee6f91 into KSP-CKAN:master Dec 28, 2017
politas added a commit that referenced this pull request Dec 28, 2017
@politas politas removed Enhancement New features or functionality Pull request ★★☆ labels Dec 28, 2017
@Olympic1 Olympic1 deleted the feature/RightClickMenu branch December 28, 2017 12:44
@Olympic1 Olympic1 removed the In progress We're still working on this label Dec 28, 2017
@politas politas mentioned this pull request Jan 5, 2018
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.

3 participants