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
Show uninstall text even with /x /qf #1881
Conversation
@barnson this is strange, but when I use the I'm trying to make one more change to this, so we might want to hold off @danieljurek. If you uninstall e.g., Could also go straight to the maintenance dialog, but I need to play around with it more. I'll mark this as a draft for now. I don't think the experience is great as it is here in this PR now. The other changes I mention above are on my system now only. |
Internal UI should never be shown externally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preventing accidental merge. This will require some more testing.
@barnson can you clarify? I'd be displaying custom dialogs externally only, but still won't fix the issue where the default uninstall mode (sans We can't really do anything about that, @danieljurek. MSI just completely disables dialogs in that scenario. We can, however, override ARP so that we uninstall with a custom string instead of the default handling by ARP itself. This is actually very common when using chainers. Can still provide a repair experience, IIRC. |
My generic complaint about the limitations of internal UI. It and things like state prefixes aren't well specified, so it's down to observed behavior (which we then forget). |
@danieljurek we may want to consider overriding ARP so that we can prompt to uninstall using UI mode. The default ARP does is basic. This is easy to do, and the norm for installers using a chainer. I could open a separate issue on that if you agree this scenario is worth the effort. |
@danieljurek ping |
/azp run azure-dev - cli |
Azure Pipelines successfully started running 1 pipeline(s). |
@danieljurek for the build failure, did the binary output move? Do you want me to fix this, or do you plan on it since it's more so build config? I'm fine either way. |
Looks like a flaky test. Re-run succeeded. Now I can test on the MSI file. |
From my testing:
@heaths -- I found what looks like a duplication of the label at the top of the dialog during uninstall: |
@Austinauth, @savannahostrowski -- This is a UX change and I want your approvals. We added text to package uninstalls for brew and Linux a while back warning the user that This adds the same message to an uninstall dialog which will appear during "fully interactive" uninstall scenarios. This happens after the uninstall of Right now this change only affects users who specifically uninstall the MSI using the The default experience for the |
I can't see all the steps in the wizard, but the only thing I would suggest is adding a message somewhere to make it completely clear the uninstall was successful. If this happens in a prior step, no worries. If not I suggest: Option 1: Modify the primary messageOriginal Proposed Option 2: Modify the first sentenceOriginal Proposed |
But as the thread above indicates, I can - and believe I should - change that relatively easily. Most users are going to use Add or Remove Programs (ARP) to uninstall, mostly likely. It's not hard. I'll include it in this PR. What I will not do is make a custom UI pop in non-full UI mode because that breaks expectations, automation, and guidelines. Modifications like dialog text can and only should be done in full UI mode. |
I fixed the progress dialog (not straight forward at all because of the resequencing, so you'll see some comments explaining why some seemingly strange authoring is there) and the uninstall dialog now looks like: I've also made it so that uninstalling from Add or remove programs displays the full uninstall UI. |
I think that wording wise, this is fine. I agree with Austin's suggestions. I'm assuming there's no way to stylize the paths/commands in this text? |
No. I can't stress enough how limited the MSI UI is. It's all basic win32 controls, and older versions at that. For that reason - and for multi-package products - is why most larger products ship a chainer with a fully custom UI, but that would greatly increase your download size over what is really small right now, and I don't recommend it. It's a large burden with no significant payout when most acquisition stories won't show your UI anyway. |
@heaths wrote:
Depends on the chainer; Burn's about 1MB, 1/8th of the .msi. |
Fair, given it's a Go binary which tend to ship a large runtime. But the point about not being seen as often - especially uninstall (not many do that) - and the cost of writing a completely custom UI only for Windows when other acquisition methods never show a UI (and on Windows, you typically wouldn't see it using scoop, choco, or winget) is not something I'm willing to tackle on personal time. |
Sure, just talking about size. 1MB can be significant for a single, small package but at a certain point, it's lost in rounding. And you can control the uninstall UI. :) |
@danieljurek is this ready to merge? |
/azp run azure-dev - cli |
Azure Pipelines successfully started running 1 pipeline(s). |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Finally had a chance to come back to this. Uninstalling from ARP -- dialogs are: <progress dialogs...> @Austinauth -- This now includes your Option 1 suggestions. Testing:
I'm good to merge this. @Austinauth, @savannahostrowski for uninstall UX changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #1858