Skip to content

Improve Message Box in WinForms #13440

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

memoarfaa
Copy link

@memoarfaa memoarfaa commented May 11, 2025

Fixes #12044

Proposed changes

  • Style, localize Message Box and add some public API to use custom icon.

Screenshots

Before

لقطة شاشة 2025-05-11 190703

After

لقطة شاشة 2025-05-11 190944

Untitledlast

Test methodology

Manually tested via scratch project.

Microsoft Reviewers: Open in CodeFlow

Copy link

codecov bot commented May 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 141 lines in your changes missing coverage. Please review.

Project coverage is 76.58368%. Comparing base (72ee6eb) to head (925fc08).
Report is 3 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #13440         +/-   ##
===================================================
+ Coverage   75.43847%   76.58368%   +1.14521%     
===================================================
  Files           3230        3230                 
  Lines         639097      639238        +141     
  Branches       47289       47308         +19     
===================================================
+ Hits          482125      489552       +7427     
+ Misses        147952      146112       -1840     
+ Partials        9020        3574       -5446     
Flag Coverage Δ
Debug 76.58368% <0.00000%> (+1.14521%) ⬆️
integration 18.79525% <0.00000%> (?)
production 50.97896% <0.00000%> (+2.56687%) ⬆️
test 97.40411% <ø> (ø)
unit 48.37502% <0.00000%> (-0.03707%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dotnet-policy-service dotnet-policy-service bot added the waiting-author-feedback The team requires more information from the author label May 12, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label May 12, 2025
@Tanya-Solyanik Tanya-Solyanik added area-DarkMode Issues relating to Dark Mode feature and removed area-Infrastructure labels May 12, 2025
@KlausLoeffelmann
Copy link
Member

Hey guys,

so, I am a bit confused.
In the discussion to the issue, the sentiment seemed to be not taking any changes for the old MessageBox, let alone add new APIs to it.

So, it's not exactly clear to me, what we want to do here.
We cannot change the API of the existing MessageBox, as it would break the world, almost literally.

But we will also not have an additional MessageBox, which would do essentially do the same as the old one. We said that we want the TaskDialog to move forward, and I am looking actively at what options we might have, to have that one adapted to dark mode.

Yes, it's not ideal that we do not have dark mode support for the native MessageBox-es as it is.
And even if we did not touch their APIs - looking at the code, I am not entirely sure, if we want to take the risk of hijacking too many of the API calls (but @JeremyKuhne would have the final saying in this, anyway) as a principal approach.

The only way to get the old MessageBox display acceptable in dark mode would probably be, to completely reimplement it in WinForms based on what we're actually supporting in terms of dark mode support.

That's something I had considered, and I have actually a prototype for testing, and would bring TO a team meeting if time allows during the Preview 6-time frame. But even that is not really likely to make the bar.

We need to discuss internally at the next team meeting, and I will follow up, but I do not see a good chance this being taken.

Klaus

@KlausLoeffelmann KlausLoeffelmann added the untriaged The team needs to look at this issue in the next triage label May 12, 2025
@AraHaan
Copy link
Member

AraHaan commented May 12, 2025

Hey guys,

so, I am a bit confused. In the discussion to the issue, the sentiment seemed to be not taking any changes for the old MessageBox, let alone add new APIs to it.

So, it's not exactly clear to me, what we want to do here. We cannot change the API of the existing MessageBox, as it would break the world, almost literally.

But we will also not have an additional MessageBox, which would do essentially do the same as the old one. We said that we want the TaskDialog to move forward, and I am looking actively at what options we might have, to have that one adapted to dark mode.

Yes, it's not ideal that we do not have dark mode support for the native MessageBox-es as it is. And even if we did not touch their APIs - looking at the code, I am not entirely sure, if we want to take the risk of hijacking too many of the API calls (but @JeremyKuhne would have the final saying in this, anyway) as a principal approach.

The only way to get the old MessageBox display acceptable in dark mode would probably be, to completely reimplement it in WinForms based on what we're actually supporting in terms of dark mode support.

That's something I had considered, and I have actually a prototype for testing, and would bring TO a team meeting if time allows during the Preview 6-time frame. But even that is not really likely to make the bar.

We need to discuss internally at the next team meeting, and I will follow up, but I do not see a good chance this being taken.

Klaus

I think installing a hook and uninstalling the hook inside of ShowCore is fine, but allowing one to be able to:

  • customize the colors beyond just going into dark mode.
  • customize the icon.

Are both beyond the scope of MessageBox.

However, hooking into it's WndProc to override it into dark mode should be alright as long as the person who opened this PR makes it to where that is not part of the public API surface (to ensure existing code does not break) while also allowing dark mode out-of-box when the developer enables dark mode in their application. The reason for this is that I see a way that this can be done as an implementation detail without exposing any new public API's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DarkMode Issues relating to Dark Mode feature untriaged The team needs to look at this issue in the next triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Suggestion] Improve MessageBox in WinForms dotnet Core
4 participants