Skip to content

CE custom changes for Winsparkle (background checking, UI changes)#2

Open
jaredburkeen wants to merge 1 commit intomasterfrom
ce-winsparkle-stable
Open

CE custom changes for Winsparkle (background checking, UI changes)#2
jaredburkeen wants to merge 1 commit intomasterfrom
ce-winsparkle-stable

Conversation

@jaredburkeen
Copy link
Copy Markdown

@jaredburkeen jaredburkeen commented Oct 10, 2016

This pull request replaces an earlier pull request:
#1

This new request is based on the stable release 0.5.2 of Winsparkle. The prior request was based off of master, which had a bug which resulted in a crash when downloading the update multiple times:
vslavik#117

@jaredburkeen
Copy link
Copy Markdown
Author

@loberlander This is ready too, you've seen this in a prior pull request.

Comment thread src/ui.cpp Outdated
m_updateButtonsSizer->Add
(
m_installButton = new wxButton(this, ID_INSTALL, _("Install update")),
m_installButton = new wxButton(this, ID_INSTALL, _("Download")),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just for consistency, wouldn't it be better to rename ID_INSTALL to ID_DOWNLOAD?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, that makes sense.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You might also want to rename any related items like OnInstall().

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sounds good, thank you.

Comment thread src/ui.cpp
{
Settings::WriteConfigValue("SkipThisVersion", m_appcast.Version);
static const int ONE_HOUR_IN_SECONDS = 3600;
Settings::WriteConfigValue("UpdateInterval", ONE_HOUR_IN_SECONDS);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a default for UpdateInterval? I.e. what happens when neither of the Remind buttons are pushed and the X (close window) is used instead?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The default is what we set in our application when we initialize Winsparkle (24 hours).
Beyond that, if the user clicks on Remind in an hour, then the next clicks the Close button, the update interval will be 1 hour. I explained this behavior to Amber and she was OK with it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am good with this as long as it is UX approved. 🖖

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