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

Sort by "update"-column on clicking "add available updates"-button #2392

Merged
merged 5 commits into from
Apr 11, 2018
Merged

Sort by "update"-column on clicking "add available updates"-button #2392

merged 5 commits into from
Apr 11, 2018

Conversation

DasSkelett
Copy link
Member

@DasSkelett DasSkelett commented Mar 27, 2018

Hey, I just thought of a little enhancement, and decided to create the PR myself.

So now, after clicking the "Add all available updates" button, the modlist automatically sorts by "update" column and focuses/selects and scrolls to first row.

I'm not that experienced at programming and furthermore this is my first commit on CKAN and I just copy-pasted together code which seemed to do the right things, after trying to get an overview of the code for some hours :D

Well, it worked after building, so yeah.

…ates"-button

Automatically sort by "Update"-column after clicking "Add available Updates"-button

Verified

This commit was signed with the committer’s verified signature.
@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI labels Mar 28, 2018
@politas
Copy link
Member

politas commented Mar 29, 2018

This seems like something that should be an option. I'm sure some users would find it extremely annoying, I'd be happy for it to be the default behaviour, though. I realise that this would bump up the complexity of the coding significantly, so @DasSkelett, if you'd like some help with that, just say so and I'm sure someone would be happy to make a PR to your branch to handle the option-setting.

@DasSkelett
Copy link
Member Author

Thanks for your thoughts.
I started to add an checkbox in the settings GUI, I think I can handle it myself, but it wouldn't be bad if someone reviews the code after that.

So should it be opt-in or opt-out?

@DasSkelett DasSkelett changed the title Sort by "update"-column on clicking "add available updates" WIP: Sort by "update"-column on clicking "add available updates"-button Mar 29, 2018
@politas
Copy link
Member

politas commented Apr 2, 2018

Make it opt-out.

Added a "more settings" group box and  a textbox do disable auto-sort.
@DasSkelett
Copy link
Member Author

DasSkelett commented Apr 2, 2018

Okay, I added the option in the Settings menu in a "more Settings" group box:
ckansettings

I think this is ready-to-merge

@DasSkelett
Copy link
Member Author

Okay, don't know why my upstream pull (only containing a495f43) failed CI...

@Dazpoet
Copy link
Member

Dazpoet commented Apr 2, 2018

It failed and travis posted something about tcp errors so I restarted the job and it passed... sounds like a network issue more than a codeissue.

@DasSkelett DasSkelett changed the title WIP: Sort by "update"-column on clicking "add available updates"-button Sort by "update"-column on clicking "add available updates"-button Apr 2, 2018
@DasSkelett
Copy link
Member Author

Ah thanks :D

Sorry, something went wrong.

@politas
Copy link
Member

politas commented Apr 3, 2018

Could you move the "Hide Epochs" button into the new "More Settings" group box? It doesn't really belong in Updates.

Sorry, something went wrong.

Move "Hide Epochs" checkbox to "More Settings" group box.
@DasSkelett
Copy link
Member Author

DasSkelett commented Apr 3, 2018

Moved it, now looking like this:
ckansettings


One more question:
Why are most controls of the AutoUpdateGroupBox depending on AutoUpdate.CanUpdate?
Can't we just block the InstallUpdateButton, if we can't overwrite the exe?
So instead:

private void InstallUpdateButton_Click(object sender, EventArgs e)
{
    if(AutoUpdate.CanUpdate)
    {
        Hide();
        Main.Instance.UpdateCKAN();
    }
    else
    {
        GUI.user.RaiseError("Can't autoupdate. Please check https://github.com/KSP-CKAN/CKAN/ for help!")
    }
}

This would solve the annoying Designer Error in VisualStudio and the Settings window would look nicer if AutoUpdate isn't available.


Edit: I created a new branch on my fork for my question.
Check this commit for my proposed changes.

@politas politas merged commit c8d3297 into KSP-CKAN:master Apr 11, 2018
@politas politas removed Enhancement New features or functionality Pull request labels Apr 11, 2018
@politas
Copy link
Member

politas commented Apr 11, 2018

DasSkellet, can you raise a new PR for the change in controls dependency? I'm not sure what designer error you are getting, as I don't use VisualStudio most of the time.

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.

None yet

5 participants