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

Use modern-themed buttons in the host error message box. #71288

Closed
Tracked by #77456
teo-tsirpanis opened this issue Jun 25, 2022 · 21 comments · Fixed by #78087
Closed
Tracked by #77456

Use modern-themed buttons in the host error message box. #71288

teo-tsirpanis opened this issue Jun 25, 2022 · 21 comments · Fixed by #78087
Milestone

Comments

@teo-tsirpanis
Copy link
Contributor

teo-tsirpanis commented Jun 25, 2022

The recent blog post with the apphost improvements features an error message box for the cases when the appropriate runtime was not found.

image

As seen in the screenshot, the message box uses old-style buttons. They could be made more modern pretty easily. This is how it would look like:

image

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 25, 2022
@ghost
Copy link

ghost commented Jun 25, 2022

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

The recent blog post with the apphost improvements features an error message box for the cases when the appropriate runtime was not found.

image

As seen in the screenshot, the message box uses old-style buttons. They could be made more modern pretty easily.

Author: teo-tsirpanis
Assignees: -
Labels:

area-Host

Milestone: -

@Symbai
Copy link

Symbai commented Jun 25, 2022

This has been suggested before and it has been dropped by the words of "its not worth it". Which both is also stated by @richlander in the blog post in the comment section, see https://devblogs.microsoft.com/dotnet/dotnet-apphost-improvements/#comment-15937

@teo-tsirpanis
Copy link
Contributor Author

I've seen the issue, I'm not asking for a task dialog, just the existing message box to be modernly themed.

@richlander
Copy link
Member

richlander commented Jun 25, 2022

I changed my mind based on an abundance of feedback. Let's try again for .NET 8.

I added a (currently closed) line item at #71284. I added yours there, too, @teo-tsirpanis.

@richlander
Copy link
Member

I'll leave it to @elinor-fung if she thinks something targeted can be done with the current implementation for .NET 7. I'm quite happy with the improvements we made for .NET 7 and to wait for considering a broader set of changes in .NET 8.

@reflectronic
Copy link
Contributor

Improving the appearance of MessageBox as suggested requires using ComCtl32 version 6, which is also what blocked the use of Task Dialog. @vitek-karas's workaround would work for both, but I do not think the idea was explored in full to ensure that it's appropriate for apphost.

@richlander
Copy link
Member

I could imagine custom hosts. For example, for WinUI apps, a host that loaded and used WinUI might make sense. It might not make sense for all scenarios, but might be great for some.

@teo-tsirpanis
Copy link
Contributor Author

I didn't know how big of a problem referencing ComCtl 6 would be, I thought it was something easy. I was going to say "why not do what WinForms' Application.EnableVisualStyles() does?", but then I realized that it creates a temporary manifest file on the disk, which is too much for the host in an error case.

If the host happens to have a custom manifest with ComCtl 6, it would "just work", right? In this case this issue can be closed and superseded by the bigger Task Dialogs effort.

@richlander
Copy link
Member

The host team's viewpoint is that the host shouldn't bring any additional dependencies or files. That's pretty justifiable, IMO. Custom hosts could do anything. That's basically it.

@vitek-karas
Copy link
Member

If the host happens to have a custom manifest with ComCtl 6, it would "just work", right?

Correct (although I didn't test this), you can add this manifest to your app as a native resource (and will be copied/applied to the host).

The challenge is how to enable ComCtl6 dynamically without writing to disk and only where we're about to fail. (and without needed any other files around). There are some ideas, but we haven't really looked into the details yet. As @richlander noted, #71284 adds this into the list for .NET 8 proposal.

@deeprobin
Copy link
Contributor

A fancy .NET icon on the left would be nice

@agocke agocke added this to the Future milestone Jun 30, 2022
@agocke agocke removed the untriaged New issue has not been triaged by the area owner label Jun 30, 2022
@agocke agocke modified the milestones: Future, 8.0.0 Jun 30, 2022
@richlander
Copy link
Member

An easy first step is for someone to build an app (Windows Forms, WPF, ...) that demonstrates the desired UX.

@deeprobin
Copy link
Contributor

An easy first step is for someone to build an app (Windows Forms, WPF, ...) that demonstrates the desired UX.

How about something like this?
grafik
DotnetHost.zip

@elinor-fung
Copy link
Member

elinor-fung commented Sep 7, 2022

I was actually playing with this a bit last week too - had something somewhat similar. Didn't try anything with the .NET icon, since we don't include that with apphost to keep its size down and I don't think we want to introduce it just for this error.

Newer visual style via embedded manifest (not as exe/dll manifest) and activation context:
+1kB to Windows apphost
image

TaskDialog (also requires the activation context) - I think we'd need this to get a clickable link:
+2kB to Windows apphost:
image

Thoughts?

@deeprobin
Copy link
Contributor

Didn't try anything with the .NET icon, since we don't include that with apphost to keep its size down and I don't think we want to introduce it just for this error.

The host size is basically decisive in the single file host.
For the normal apphost, I think we should prioritize convenience/UX over size.

TaskDialog (also requires the activation context) - I think we'd need this to get a clickable link:
+2kB to Windows apphost:

Looks also good to me. But I like the icon ;)

@vitek-karas
Copy link
Member

Agree that we should not include the icon due to its size.
If we were to use TaskDialog I would remove the red cross icon - feels too jarring to me. I think a big (which this will be) dialog clearly explaining what the problem is looks better.
With TaskDialog I would also include the link the dialog redirects to in its text as a link (getting the URL the dialog points to is tricky since when opened it immediately redirects and it might be useful).

I'm obviously a terrible designer, but if we want nicer, we could do something like this:
image

And this is how it looks when expanded:
image

@elinor-fung
Copy link
Member

Another variation could be to keep the things like architecture and version in the main content and have the 'see details' only include the full download link. Or show it with the details expanded by default (but the full - usually long - link shown by default might be a bit noisy). I know we've gotten feedback in the past about wanting to have architecture/version clearly called out, so it might be good to keep those shown by default.

@vitek-karas
Copy link
Member

That sounds good as well - I was kind of hoping that the architecture/version should be handled by the website - since mostly it's needed to pick the right URL on the existing website. But either way works great.

@elinor-fung
Copy link
Member

#76762 has just the visual style / modern-themed buttons portion of this. It is the change in the issue's original description and would be a prerequisite for using the win32 task dialog (for clickable links, download button, expandable details, etc.)

@elinor-fung elinor-fung mentioned this issue Oct 27, 2022
10 tasks
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 12, 2023
@TheDusty01
Copy link

I know this already has a PR but the new TaskDialog style fits more to Windows 7 or 8 than to 10 and 11 (looks way better than the dialog before though).

Since the goal is to make the dialog look more modern, WinUI 3 would be better here imho. It fits more to Windows 10 & 11 and also has the most modern style.
I'm curious, is there a reason on why this isn't considered or possible?

A sample WinUI 3 MessageDialog:
image

@elinor-fung
Copy link
Member

Since the apphost that shows the dialog is part of every .NET application, one of its basic tenets is to be as lightweight as possible. WinUI 3 uses the Windows App SDK runtime and would mean deploying and bootstrapping those runtime packages. That's not a requirement/dependency we'd want, so WinUI is not really an option for the host, unfortunately.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 18, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants