Skip to content

Conversation

@bergmeister
Copy link
Contributor

PR Summary

Move definition of Launch PowerShell checkbox to the top to allow toggling it using the spacebar (the Enter key already defaults to the Finish button).
This is quite common amongst best practice WiX installers to make it possible to navigate through the dialogs without having to touch the mouse. This behvaiour is the same as the licence dialogue that allows toggling the checkbox using the space bar as well,
This does not affect the visual appearance.
Also reuse product name variable

PR Checklist

Note: Please mark anything not applicable to this PR NA.

…e Enter key already defaults to the `Finish`) button.

Reuse product name variable
@bergmeister bergmeister changed the title Windows installer: Allow Launch PowerShell checkbox to be togged using the space bar (the Enter key already defaults to the Finish button) Windows installer: Allow Launch PowerShell checkbox to be toggled using the space bar (the Enter key already defaults to the Finish button) Jan 5, 2018
@iSazonov iSazonov self-assigned this Jan 5, 2018
@iSazonov iSazonov requested a review from SteveL-MSFT January 5, 2018 18:26
@iSazonov
Copy link
Collaborator

iSazonov commented Jan 5, 2018

It works. But I caught another problem: I install current build PowerShell-6.1.0-preview.7428-win-x64.msi over previous PowerShell-6.1.0-preview.7420-win-x64.msi and lost the local Explorer menu for PowerShell 😕

@bergmeister
Copy link
Contributor Author

bergmeister commented Jan 5, 2018

@iSazonov : I cannot repro your problem. I installed 6.1.0-preview.7420-win-x64 first and then 6.1.0-preview.7436-win-x64 over it and the context menu is still there. Are you sure you did not forget to tick the checkbox again when installing it the 2nd time? Maybe it is missleading that the checkbox says Add context menu and the word Add should maybe be replaced with something like Configure. I might be able to change the behaviour but I would like to get clarification first what the expected behaviour should be when not checking the checkbox the 2nd time. I think what happens behind the scenes is that the upgrade uninstalls some things first (leading to the removal of registry entries) and then does the upgrade installation (one could almost say, it is a feature not a bug 😜).
One could argue for and against either case, e.g.:

  • Person A wants to upgrade PowerShell but this time decided (s)he does not want the context menu any more, so he/she does not tick the checkbox to get rid of it. In this case the current behaviour would be nice because otherwise the person has to uninstall and re-install just to get rid of the context menu.
  • Person B wants to only upgrade PowerShell but keep the current configuration without having to remember the configuration. In this case the current behaviour is not good.

The only way to fulfill both cases would be to detect that it is an upgrade install and set the defaults to the current configuration. This might be quite difficult and error prone to implement.

@SteveL-MSFT
Copy link
Member

@bergmeister I would defer worrying about those cases until there is sufficient feedback we need to support it (whichever case)

@iSazonov iSazonov merged commit e351a7e into PowerShell:master Jan 6, 2018
@iSazonov
Copy link
Collaborator

iSazonov commented Jan 6, 2018

@bergmeister Never mind, I used non-rebased branch.

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.

3 participants