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

Switch installer to use classic theme #238

Merged
merged 7 commits into from
Jan 27, 2016
Merged

Switch installer to use classic theme #238

merged 7 commits into from
Jan 27, 2016

Conversation

gbiellem
Copy link
Contributor

The "Black Surface" theme used in the ServicePulse Installation is broken out of the box. See #233

This PR is to switch the theme to the classic theme with Particular specifiic logos

@gbiellem gbiellem changed the title [WIP'] Switch installer to use classic theme [WIP] Switch installer to use classic theme Dec 14, 2015
@gbiellem gbiellem changed the title [WIP] Switch installer to use classic theme Switch installer to use classic theme Dec 14, 2015
@gbiellem
Copy link
Contributor Author

@petersgiles @sergioc

Can you take a look at this and see if you are happy with it. The resultant installer for this branch can be found here: - http://builds.particular.net/repository/download/ServicePulse_1NewBuil/137556:id/assets/Particular.ServicePulse-1.4.0.exe

@gbiellem
Copy link
Contributor Author

@sergioc can you confim that the background on the SP_installer_initial.jpg is supposed to be a light grey. The other ones you provided in the zip are white.

@sergioc
Copy link
Contributor

sergioc commented Dec 14, 2015

@sergioc can you confim that the background on the SP_installer_initial.jpg is supposed to be a light grey.

@gbiellem you're right, I fixed it and made the background white.

@sergioc
Copy link
Contributor

sergioc commented Dec 14, 2015

Regarding the rest of the installer, I see 2 issues with the following screen:

2015-12-14 at 07 55

  1. The Back button is disabled and I see no reason for it to be disabled.
  2. The text "Instance URI" is cut off at the bottom

In addition to the above, I'm wondering about the doco that pops-up automatically after the installation. I don't think it's a good follow-up to installing SP. A general intro to SP would make more sense, I think. But since we don't have that, how about we show on the last screen a list with the following links (which together form the intro to the main features):

Useful documentation articles to get started:

@gbiellem
Copy link
Contributor Author

Thanks @sergioc

  • Not sure why back is disabled - Will fix
  • Don't see the text cut off on my machine, I'll adjust it and get you to retest.

The installer actually pops up http://www.particular.net/Installation-completed?installer=ServicePulse&version=[ProductVersion] So the page you don't want displayed is not hard coded in the installer - it's a redirect from the main site. I'm fine with adding links to the installer "installation complete" screen if we pull out the web popup but I assume that would also affect any stats collection we do on that page. I'd also want to do this consistently on all the installers

@sergioc
Copy link
Contributor

sergioc commented Dec 14, 2015

@gbiellem the reason I suggest the list of links is because the page that opened automatically for me from http://www.particular.net/Installation-completed?installer=ServicePulse&version=%5BProductVersion%5D was http://docs.particular.net/servicepulse/intro-endpoints-heartbeats

Is that the page that is supposed to open?

@gbiellem
Copy link
Contributor Author

@sergioc AFAIK it's been that way since V1. I think your original comment about there not being a general intro into SP is the the reason it goes there, it's the closest thing there is to an overview.

@sergioc
Copy link
Contributor

sergioc commented Dec 14, 2015

@gbiellem I opened an issue requesting an SP intro doco: Particular/docs.particular.net#976

@gbiellem
Copy link
Contributor Author

@petersgiles @sergioc - Take 2. Please review
The resultant installer for this branch can be found here: - http://builds.particular.net/repository/download/ServicePulse_1NewBuil/138049:id/assets/Particular.ServicePulse-1.4.0.exe

@gbiellem gbiellem self-assigned this Dec 16, 2015
@sergioc
Copy link
Contributor

sergioc commented Dec 16, 2015

Few more comments:

  1. Product EULA mentions "NServiceBus license agreement" instead of Particular Software:

screenshot 2015-12-16 09 12 13

  1. "Folder:" text shows below path

2015-12-16 at 09 13

  1. I have no clue why it says "Advanced installer" at the bottom, it seems redundant. I'd remove that to prevent it from raising questions:

2015-12-16 at 09 13 1

@gbiellem
Copy link
Contributor Author

@sergioc

  1. All of the license agreements say "NServiceBus" these come from @SamCamhi so I assume there is a legal reason for this. Remember we are NServiceBus Ltd. doing business as Particular Software.
  2. Oops - will fix. Not sure why it's off since that's an OOTB screen I haven't updated.
  3. This is just some over zealous marketing in the advanced installer templates. They've actually made it quite difficult to remove. I checked the SC installer and it's present there as well. I'll see how much effort it is to pull out but frankly i think it's fine to stay there as is.

@gbiellem
Copy link
Contributor Author

Since the "Advanced Installer" watermark is part of the template and not the project I've had to hack every screen to remove it. This works fine but it means it anyone adds a new screen they'll have to remember to do the same.

@gbiellem
Copy link
Contributor Author

@petersgiles
Copy link
Contributor

I'm happy with it

@sergioc
Copy link
Contributor

sergioc commented Dec 17, 2015

@gbiellem is it possible to remove the "modify" option from the installer, when SP is already installed? It doesn't make sense to have it there since there's no additional customization possible besides removing it:

screenshot 2015-12-17 10 47 56

The rest of the installer looks good. (I'll discuss the EULA issue with @SamCamhi separately)

@gbiellem
Copy link
Contributor Author

@sergioc - yes it is. I'll tweak that in the morning

@gbiellem
Copy link
Contributor Author

@gbiellem
Copy link
Contributor Author

Please review and merge if you have no other issues.

@sergioc
Copy link
Contributor

sergioc commented Dec 18, 2015

@gbiellem a few questions:

  1. Was your intention to remove all options to manage the installation or you tried to only remove the Modify option? I'm asking because when I choose to uninstall from Programs&Features, it immediately starts uninstalling after authorizing the UAC, there's no confirmation prompt (we either need that, or the other options screen without the modify option).

  2. When the uninstallation completes, there's no confirmation that it successfully uninstalled. It'll just silently exit.

  3. Sometimes after uninstalling it display this:

screenshot 2015-12-18 08 07 44

  1. After SP uninstalls, it leaves behind its entry in the Programs&Features list.

@gbiellem
Copy link
Contributor Author

@sergioc

  1. No that's a bug. Will fix
  2. That's the default when uninstalled via add/remove programs but not when the original uninstall is used. I have not found an acceptable way around that yet
  3. Can't do anything about that one. That's a built in Windows feature we have no control over
  4. It's a known issue with Add/Remove programs and not isolated to our installer. Refresh the screen with an F5 or close and open and you will see it's actually not there.

@gbiellem
Copy link
Contributor Author

@sergioc I'll have to put this on hold until we upgrade Advanced Installer. We're currently on 10.6 and have no maintenance. Some of the problems I'm hitting getting this working the way you want were addressed in 11.1. I'll ping @SamCamhi and see if we can update.

@sergioc
Copy link
Contributor

sergioc commented Dec 21, 2015

@gbiellem 👍

@SamCamhi
Copy link
Member

@gbiellem updated maintenance. Also, got a 6 month free trial for another product. Sent you license key and info

@gbiellem
Copy link
Contributor Author

Awesome @SamCamhi - thanks

@petersgiles
Copy link
Contributor

@gbiellem can we merge this one now

@gbiellem
Copy link
Contributor Author

Not yet. I parked this until the build agents were updated to V12 of Advanced Installer
That's done now so I guess I should finish it off.

@gbiellem gbiellem added this to the 1.3.7 milestone Jan 27, 2016
petersgiles pushed a commit that referenced this pull request Jan 27, 2016
@petersgiles petersgiles merged commit af27dcb into develop Jan 27, 2016
@petersgiles petersgiles deleted the update-Install branch January 27, 2016 23:19
@petersgiles petersgiles removed this from the 1.3.7 milestone Jan 28, 2016
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.

5 participants