dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.4k stars 977 forks source link

Add IsAncestorSiteInDesignMode for Control #4146

Closed KlausLoeffelmann closed 3 years ago

KlausLoeffelmann commented 3 years ago

Rationale

WinForms Developers need a more reliable way to check, if a custom control is currently in DesignMode. The Problem is that the DesignMode property of Control does only return true, if that control is a) actually sited, and b) then returns the DesignMode property of the Site.

In many cases, DesignMode that does not help: Whenever a control is part of a constituent control (like the TextBox control in NumericUpDown) or a UserControl, the individual controls making up the UserControl (constituent control) never get sited, and although there is a Site technically which could return true for DesignMode, from the perspective of that control, that never happens.

IMPORTANT NOTE: This API's purpose is not to provide a way to check, if a control is currently hosted by a Designer for License purposes, so for them to decide, if for example the Designer UI can be made accessible, if a necessary license is present. We would need a different infrastructure for that.

Proposed API:

Add a read-only property to Control, which returns a boolean:

public bool IsAncestorSiteInDesignMode

IsAncestorSiteInDesignMode would find the first sited root (parent/ancestor) control, test for that being sited and return DesignMode, thus a rather reliable way for a control to decide if according to its context it is considering itself to be in design mode (per the WinForms definition for DesignMode).

Will this feature affect UI controls?

No.

Practically, it could be looking something like this:

        protected bool IsAncestorSiteInDesignMode =>
            GetSitedParentSite(this) is ISite parentSite ? parentSite.DesignMode : false;

        private ISite GetSitedParentSite(Control control) =>
            control is null
                ? throw new ArgumentNullException(nameof(control))
                : control.Site != null || control.Parent is null
                    ? control.Site
                    : GetSitedParentSite(control.Parent);

Real World Scenario

Let's assume we have a custom control, which uses resources, but those resources should only be used at runtime (like connections to a database, or a connection to a web service).

That means, when ever not only this custom control is in its DesignMode...

image

...but also one of it's parents is in their DesignMode...

image

...it should not touch those resources.

As soon, however, as it detects that none of its ancestors is sited (or their sites are not reporting they are in DesignMode) it can assume it's safe to use those runtime-only resources:

image

From the custom control's point of view, such an implementation would look like this:

    class CustomControl : Control
    {
        private Brush _cashedBackColorBrush = new SolidBrush(Color.DarkGray);
        private bool _allowRunTimeOnlyResources;

        protected override void OnHandleCreated(EventArgs e)
        {
            base.OnHandleCreated(e);
            if (!IsAncestorSiteInDesignMode) 
            {
                _allowRunTimeOnlyResources = true;
            }
        }

        protected override void OnPaint(PaintEventArgs e)
        {
            string drawText;

            base.OnPaint(e);

                e.Graphics.FillRectangle(_cashedBackColorBrush, ClientRectangle);
                var canOrNot = _allowRunTimeOnlyResources ? "can" : "should not";

                drawText = $"{this.Name} {canOrNot} use runtime resources currently.";
                CenterString(drawText);

            void CenterString(string paintText)
            {
                var textSize = e.Graphics.MeasureString(paintText, Font);

                e.Graphics.DrawString(paintText,
                    Font,
                    Brushes.Black,
                    ClientSize.Width / 2 - textSize.Width / 2,
                    ClientSize.Height / 2 - textSize.Height / 2);
            }
        }
    }

Note, that the test for being sited or not cannot be done in the custom control's constructor. Because at that time, neither the control nor its ancestor can be sited (or rather, it will probably not have ancestors at the time, since it has not been added to any control collection). So, the earliest point in time, when a custom control should check if it has been sited is after its handle got created. And since its handle gets created once the control should display its contends, in most of the cases this is an early enough point in time to decide, whether to use certain (runtime-only) types of resources or not.

RussKie commented 3 years ago

I think this closely relates to https://github.com/dotnet/winforms/issues/3300. In my opinion the key problem with "design-time" check is that everyone has different understanding of "design time". @weltkante has laid it out nicely in https://github.com/dotnet/winforms/issues/3300#issuecomment-708365820.

With this I think we need to break down different user scenarios, and walk from there:

  1. A user control is being designed - this is what DesignMode does. We definitely need to improve the docs to clearly articulate the purpose of this property, and describe how to use it.
  2. A user control has been placed on another control in the designer - a lot of user (myself included until recently) expect DesignMode to return true, but generally this returns false. This is what #3300 is asking for, and IIUIC this is what your proposal is for.
  3. A user control is shown when an app is run (added this for completeness).

I don't think a lot of users will have a clear idea what Site is (our docs are pretty useless if we try to find out more about it), and this makes the name IsRootSiteInDesignMode almost meaningless for an average Joe developer.

KlausLoeffelmann commented 3 years ago

I struggled a bit with the name as well, but in the end I thought it also not a typical Joe developer scenario to begin with. Those who need it know what they are missing, and the name is honest. On top of that, I think the new API would get around comparatively quickly.

RussKie commented 3 years ago

it also not a typical Joe developer scenario to begin with

I disagree here, in my experience developers quite often want to do something funky at the runtime but not in the designer. GitHub shows 70K+ hits: https://github.com/search?l=C%23&q=DesignMode&type=code

RussKie commented 3 years ago

Browsing the GitHub I stumbled upon an implementation which is doing the same what you are proposing here - https://github.com/k3ralas/Zoo.Serialization/blob/0734d54f005f2d677d350bef2269ff4818085938/ZooRunner/ZooRunner.GUI/ControlExtensions.cs#L3

There are quite a few copies of this file in multiple repos.

weltkante commented 3 years ago

While the API you suggest is easy to implement it won't quite cover the use cases from #3300, namely checking DesignMode in the constructor. Many people already want to know in the constructor if the control is going to be created for the designer or for the runtime and currently were using the licensing API for that as a workaround, but apparently this workaround broke in .NET 5.

As I mentioned in the issue other .NET UI Frameworks have an API for a static design time check not relying on siting, so I'd strongly suggest WinForms to follow the precedence and not invent yet another semantic, otherwise you still have to face the problem that the workaround using the licensing system design time flag is broken.

KlausLoeffelmann commented 3 years ago

namely checking DesignMode in the constructor.

What people think they want is one thing, what they need another. There could be a high chance that the wrong didactic in the documentation and discoverability is the real issue here.

So, let’s discuss the actual scenarios where people need this in the constructor, so the „and I need this because...“ cases:

IrinaPykhova commented 3 years ago

I need it to determine that arbitrary control regardless of whether it is used by its own or inside other control is used in IDE designer, not in running application. Maybe I want to initialize some control properies differently in designer and runtime. Or maybe I don't want to run timers or background workers which only make sense for runtime. There are a lot of different scenarios. Currently I need to do the check before control is sited and finished with crazy code like this: string ent = Assembly.GetEntryAssembly().FullName; if ((ent.StartsWith("DesignToolsServer") && ent.EndsWith("PublicKeyToken=31bf3856ad364e35")) // VS designer

Why don' have something like this https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.designerproperties.getisindesignmode?view=netcore-3.1 in WinForms?

KlausLoeffelmann commented 3 years ago

Why don' have something like this https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.designerproperties.getisindesignmode?view=netcore-3.1 in WinForms?

Because we have the concept of a DesignerHost, which means we don't have the WinForms Designer per se, which is in VS, but we could have many different designers. Also, we cannot or shouldn't break existing scenarios, so detecting if the Control is sited and if that Site is in DesignMode should still be the way to go in principle.

There are a lot of different scenarios.

Sure. Let's find and discuss the important ones in particular, and let's then find out, if we really need to know already in the constructor, if a control is in design mode. My assumption is that things will work...

Let's not be vague, but concrete in the discussion.

Currently I need to do the check before control is sited and finished with crazy code like this: string ent = Assembly.GetEntryAssembly().FullName; if ((ent.StartsWith("DesignToolsServer") && ent.EndsWith("PublicKeyToken=31bf3856ad364e35"))

I don't think that this is necessary, but we would need to discuss the concrete use case.

Currently I need to do the check before control is sited and finished with crazy code like this:

string ent = Assembly.GetEntryAssembly().FullName;
if ((ent.StartsWith("DesignToolsServer") && ent.EndsWith("PublicKeyToken=31bf3856ad364e35")) // VS designer

You should be aware, that this code has a chance to break in the future, because we cannot guaranty that that name will stay the same. (I think you know this already, I just want to point it out, since it's important.)

KlausLoeffelmann commented 3 years ago

Browsing the GitHub I stumbled upon an implementation which is doing the same what you are proposing here - https://github.com/k3ralas/Zoo.Serialization/blob/0734d54f005f2d677d350bef2269ff4818085938/ZooRunner/ZooRunner.GUI/ControlExtensions.cs#L3

@RussKie: Yes, and I think that is the reason we should have this on board. This solution also provides the Best Practice Guide, and I think having that and the correct accompanying documentation, we'd be covering 90% of all use cases. The documentation pointing out the different use cases is as important, though.

KlausLoeffelmann commented 3 years ago

We're (WinForms Team) discussing naming.

weltkante commented 3 years ago

RootDesignMode (we like that one!)

I like that one as well, but note that your implementation stops at the first site, so naming it Root is lying a bit. Sites are sometimes (very rarely) used in other scenarios than designers, and in that case you may stop early and not determine the DesignMode of the root control.

You could change your lookup code to actually compute the "root site", or leave as-is and live with the fact that it will behave weirdly if sites are used for anything but designers

Alternatively you may want to change the algorithm entirely and not look at any single site, but compute the 'logical or' of all parent sites, i.e. determine if any site in the parent chain is in DesignMode, not just the first or last site. The lookup can short-circuit and return as soon as any site is in design mode. Looping up to the root if none is in design mode is the same as would happen with the originally proposed algorithm.

IrinaPykhova commented 3 years ago

@KlausLoeffelmann I'm absolutely sure that my solution is bad, that's why I want to see more solid way to determine design-time environment. Suggestion about testing in OnPaint is awful. OnPaint should only paint. And what if it is UI-less component? Personally, when I implement some control, I usually want to do heavy initialization before painting or creating handle, so that end-user doesn't see any delays or blinking UI. But let's look at generic case, imagine that customer should have proper license to use your control. In old days MS recommended to do license check in control constructor: https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.licenseprovider?view=netcore-3.1 We ship a lot of controls all following this logic. We changed licensing mechanism for .Net 5 but still check license in ctors. Now, I need a way to understand whether I should only check runtime license, or control is used in designer and I can check local machine file system or registry. Should I change every our control and move license checks to other places? And then test whether it makes the hole allowing using our controls without license? Do you see that as real use case? My guess is that author of https://github.com/dotnet/winforms/issues/3300 also tried to check license

weltkante commented 3 years ago

The reworked licensing model is independent from the API proposed here (and in fact I don't know what the reworked license API actually is). I expect this to be documented with the rest of how to write custom design-time integration (which has been said to appear soon, but maybe you can get someone to explain it here as well to simplify the discussion).

IrinaPykhova commented 3 years ago

I tried to show you the real use case where I need to check design-time before control is sited. I beleive someone above wanted to discuss real cases.

KlausLoeffelmann commented 3 years ago

Suggestion about testing in OnPaint is awful. OnPaint should only paint.

Explain this, why is it bad? If OnPaint only paints (but yet depended of it's in DesignMode or not), why is that a bad thing?

And what if it is UI-less component?

Then obviously the OnPaint approach does not apply. What's your point?

so that end-user doesn't see any delays or blinking UI.

Testing for UI-Mode doesn't slow things down noticeably, here. Also, you could (should?) use double buffering to begin with to avoid flickering. But one way or the other, performance here is really not an issue. If it is, let's discuss the scenario where introducing for the test on DesignMode matters visibly/noticeably.

Now, I need a way to understand whether I should only check runtime license, or control is used in designer and I can check local machine file system or registry.

I don't know how your licensing mechanism works exactly, but I think it's totally possible that you can disable a control's design option gracefully (and not doing that constructor). I would even think from a clean architectural perspective, you even shouldn't let the constructor of the control fail (but since I am missing the implementation details, I am just assuming you do this, I of course don't really know that). I think refactoring this is totally possible, yes. If the approach however is that you don't let it fail, then I think setting up the infrastructure for making the control being aware of the DesignMode can easily be differed to a point in time, where the control is sited.

You can always ping me directly, and I would love to learn how we can support you better, migrating to Core. I just don't think that introducing this particular API is the actual issue here.

IrinaPykhova commented 3 years ago

Suggestion about testing in OnPaint is awful. OnPaint should only paint.

Explain this, why is it bad? If OnPaint only paints (but yet depended of it's in DesignMode or not), why is that a bad thing?

Because I want to understand where I am regardless of painting. I don't want 2 different checks for controls with UI and UI-less components. Imagine I have 300 controls with different behavior, I dont' want spending time on making different checks in every one of them. Most of these controls don't override OnPaint, why they should do that just for design-time check?

I'm afraid I'm loosing the thread. Are you convicend that this API doesn't make sense and not required? Can you tell, why in WPF they decided differently? As a control author I can think out workarounds, but why? In WPF it's very handy

RussKie commented 3 years ago

My guess is that author of #3300 also tried to check license

No, the control needed this check to render the control differently in the designer, hence OnPaint scenario is mentioned here.

RussKie commented 3 years ago

@KlausLoeffelmann here's an example where the check must work in a .ctor:

I'm porting Git Extensions to .NET 5.0, and trying to open a form in the designer: image

The code fails here (line:18): https://github.com/gitextensions/gitextensions/blob/master/ResourceManager/GitExtensionsControlInitialiser.cs#L16-L21

        public GitExtensionsControlInitialiser(GitExtensionsFormBase form)
        {
            ThreadHelper.ThrowIfNotOnUIThread();          // <-- fails here
            form.Load += LoadHandler;
            _translate = form;
        }

And this helper class is initialised like this because the helper must wire up to event in the form's life cycle: https://github.com/gitextensions/gitextensions/blob/f62be3d8ef0d5d82b0fd8ffb77cc97150737a455/ResourceManager/GitExtensionsFormBase.cs#L26-L32

        public GitExtensionsFormBase()
        {
            _initialiser = new GitExtensionsControlInitialiser(this);

            // ...
        }

I tried adding the following before the _initialiser is set, and it doesn't help - the designer still fails to render:

diff --git a/ResourceManager/GitExtensionsFormBase.cs b/ResourceManager/GitExtensionsFormBase.cs
index 7c5f605a0..22da49fba 100644
--- a/ResourceManager/GitExtensionsFormBase.cs
+++ b/ResourceManager/GitExtensionsFormBase.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
 using System.Linq;
 using System.Windows.Forms;
 using GitCommands;
@@ -25,6 +26,11 @@ public class GitExtensionsFormBase : Form, ITranslate
         /// <summary>Creates a new <see cref="GitExtensionsFormBase"/> indicating position restore.</summary>
         public GitExtensionsFormBase()
         {
+            if (DesignMode || LicenseManager.UsageMode == LicenseUsageMode.Designtime)
+            {
+                return;
+            }
+
             _initialiser = new GitExtensionsControlInitialiser(this);

             ShowInTaskbar = Application.OpenForms.Count <= 0;
KlausLoeffelmann commented 3 years ago

Well, sure, that fails. First of all, the LicenceManager doesn't work in the Core Designer. Basically, what's used to happen in the Framework designer is, that if a component got added (IDesignerHost.CreateComponent), between instantiating and adding the component, the old Designer would put the LicenceManager into Design mode, but only for that one operation. The new Designer doesn't do that.

Said that, I think you don't need that machanism. For most things, if you want to have something in the character of InitializeComplete, it's usually early enough to run it after InitializeComponent has been executed, but with the first chance the message loop is processed - or in other words after the class initializing(s) happened, but before anything else, and NOT in the context of the constructor.

So, take a look at the following code:

using System.Drawing;
using System.Threading;
using System.Windows.Forms;

namespace CsWinFormsCore
{
    public partial class frmBaseForm : Form
    {
        protected bool _isDesignMode;
        private SynchronizationContext _syncContext;
        private readonly SendOrPostCallback _sendOrPostCallback;

        public frmBaseForm()
        {
            _sendOrPostCallback = new SendOrPostCallback(SetDesignMode);

            InitializeComponent();

            _syncContext = SynchronizationContext.Current;
            _syncContext.Post(_sendOrPostCallback, null);
        }

        private void SetDesignMode(object _) 
            => _isDesignMode = DesignMode;

        protected override void OnPaint(PaintEventArgs e)
        {
            base.OnPaint(e);

            var drawModeText = _isDesignMode ? "Design mode" : "Runtime";

            var stringSize = e.Graphics.MeasureString(drawModeText, Font);
            e.Graphics.DrawString($"Painting {drawModeText}",
                Font,
                Brushes.Black,
                ClientRectangle.Width / 2 - stringSize.Width / 2,
                ClientRectangle.Height / 2 - stringSize.Height / 2);
        }
    }
}

Sure, if you open this Form, you won't see OnPaint rendering anything. That's OK and lies in the nature of the designer - since the Designer only constructs your Form, OnPaint doesn't exist at design time. Because frmBaseForm doesn't exist at design time. But if you inherit now from frmBaseForm and open THAT form in the (.NET 5) designer, you'll see this:

image

So, here the constructor also kicks of what in your case would be the call to GitExtensionsControlInitialiser, but the constructor code would finish first, the control/form then be sited, and only then everything else would happen (now with the correct DesignMode being reported). That should work in most of the cases without the need for too much refactoring, I think. Doesn't it?

KlausLoeffelmann commented 3 years ago

@RussKie: For discoverability and self explaining guideline, would it make sense to introduce an InitializeComponentCompleted event/an OnInitializeComponentCompleted method?

weltkante commented 3 years ago

For discoverability and self explaining guideline, would it make sense to introduce an InitializeComponentCompleted event/an OnInitializeComponentCompleted method?

Wouldn't this be too early, assuming it does what its named after and runs when InitializeComponent completes? Also how would it compose with Form/UserControl inheritance where you have multiple logically distinct InitializeComponent methods?

We already have Load events and OnLoad virtuals for Forms/UserControls, why do you think we need another one?

IMHO the WPF/WinUI approach makes the API simple while the suggestions you are providing require a good amount of understanding of WinForms internals, or significant developer education to get comfortable with a new pattern to follow. Its not wrong or impossible, just not what people expect.

Also we can't have an informed discussion without knowing what the new licensing model will be. If you allow instances to be loaded without a design time license then how do you except 3rd party control vendors to prevent usage of the control at design time? Terminate the process? Thats not great user experience for VS users if a license popup shows up and then terminates the designer process. And yes I've seen control vendors do that when they apparently didn't understand the way licensing was supposed to work in Desktop Framework.

I think it's totally possible that you can disable a control's design option gracefully (and not doing that constructor).

Thats not what design time licensing means. Control vendors who want design time licensing want to prevent people who got a runtime license to build additional applications referencing that control. This is achieved by preventing instantiation of the control. In the classic licx licensing model instantiation of the control happened both in the designer and in the licx build.

Please provide more information about the new licensing model so we can have an informed discussion about what design time checks people are needing. You ask for scenarios when at the same time the design time licensing scenario is currently broken and nobody knows how to write replacement code.

Because we have the concept of a DesignerHost, which means we don't have the WinForms Designer per se, which is in VS, but we could have many different designers.

Just FYI, no you can't, the team has removed significant amounts of code from the runtime into the VS designer, it is currently not possible to host Form/UserControl designers. You can build your own designer scenarios based on the available infrastructure, e.g. for reporting solutions, but you cannot host the WinForms designer anymore. This was declared 'by design' in various issues, unless sufficiently convincing arguments could be given to port the remaining designer code into the runtime.

The whole concept of "design time" for 3rd party designers that reuse the infrastructure is highly questionable, because its unclear what it exactly means. 3rd party designers for e.g. reporting in Desktop Framework wouldn't have used licx licensing in the first place. In general they tend to not allow direct placement of arbitrary 4th party controls anyways, especially now that many parts of the Control based WinForms designer are missing I would be surprised if that still works. They would however allow custom controls containing 4th party controls as implementation detail.

In my opinion its fine to treat 3rd party designers as "not in design mode" for new global scope APIs, because they likely aren't meant to invoke 4th party designer code anyways. (In other words, your suggested recursive lookup may actually regress 3rd party designers, if controls placed in a report suddenly think they are in design mode when they weren't supposed to.)

IrinaPykhova commented 3 years ago

couple of words about our licensing. As far as MS removed support for licenses.licx model, apparently every control vendor will have something different now. We decided to use runtime-only licensing, license is installed when people install NuGet package into application. License information is embedded into assembly manifest. Then, at runtime, if there is no valid license, we are showing some message and throwing exception in control constructor. What happens at design-time:

KlausLoeffelmann commented 3 years ago

First of all, thanks @IrinaPykhova for the insight of how your license model work!

@weltkante:

Wouldn't this be too early, assuming it does what its named after and runs when InitializeComponent completes? Also how would it compose with Form/UserControl inheritance where you have multiple logically distinct InitializeComponent methods?

I was thinking out loud, so please see this as a discussable thought rather than a concrete suggestion. If it was kicked of (by invoking), you still could have other InitializeComponent instances kicked of by the constructor, and it would still not run in the constructor's context. Secondly, I was trying to solve exactly that requirement that folks seems to need to check for DesignTime before CreateHandle and Load and such runs.

But before I continue to address additional discussion points, I'd like to get a few of our folks involved and clear a couple of things up before we continue our discussion. Give me a few days.

KlausLoeffelmann commented 3 years ago

So, after a lot of discussions internally and externally around this issue, here's what we'll do:

Base on this, we will go forward with this API.

weltkante commented 3 years ago

Base on this, we will go forward with this API.

If you discussed this internally, what is the feedback to my concerns against this API here and here? I'll quote the relevant parts:

KlausLoeffelmann commented 3 years ago

You could change your lookup code to actually compute the "root site", or leave as-is and live with the fact that it will behave weirdly if sites are used for anything but designers.

I would still go this route. It's an edge case, in my opinion. We could consider passing a flag, though, which controls this behavior.

About regressions: I am not afraid of any here, since it's new. We won't change any existing APIs. One thing is imperative, though: The docs should reflect what it exactly does and what the common scenarios for it are.

RussKie commented 3 years ago
  • from first comment

note that your implementation stops at the first site, so naming it Root is lying a bit. Sites are sometimes (very rarely) used in other scenarios than designers, and in that case you may stop early and not determine the DesignMode of the root control. You could change your lookup code to actually compute the "root site", or leave as-is and live with the fact that it will behave weirdly if sites are used for anything but designers Alternatively you may want to change the algorithm entirely and not look at any single site, but compute the 'logical or' of all parent sites, i.e. determine if any site in the parent chain is in DesignMode, not just the first or last site. The lookup can short-circuit and return as soon as any site is in design mode. Looping up to the root if none is in design mode is the same as would happen with the originally proposed algorithm.

From docs: "Sites bind a Component to a Container and enable communication between them, as well as provide a way for the container to manage its components.". In what situation would would there be a nested site? Would IsParentSiteInDesignMode be any better?

  • from second comment

In my opinion its better to treat 3rd party designers as "not in design mode" for new global scope APIs, because they likely aren't meant to invoke 4th party designer code anyways. (In other words, your suggested recursive lookup may actually regress 3rd party designers, e.g. if controls placed in a report designer suddenly think they are in design mode when they weren't supposed to.)

Since this will be the new API, I don't think we will be regressing anything here. A control developer will have to consciously use the new API, and any 3rd party designer will have to opt-in to the API as well. And between the two DesignMode and IsRootSiteInDesignMode API they should be able to work out the intent.

weltkante commented 3 years ago

From docs: "Sites bind a Component to a Container and enable communication between them, as well as provide a way for the container to manage its components.".

Exactly, custom containers are rare but not unheard of. We do have two in our application. Also during interop there may be 3rd party containers. The site provides a way to communicate out of band data/services with a container, which could be a designer, or something else.

In what situation would would there be a nested site?

We've been using this in custom containers to provide an IServiceProvider to controls, allowing them to query services from their container.

Also this is used in interop scenarios like ActiveX hosting in your own codebase. WinForms also allows 3rd party sites to provide ambient properties which I personally only know in context of interop, but maybe it can be used outside interop, no idea, but its public.

Would IsParentSiteInDesignMode be any better?

Slightly better naming, still misleading since you have recursion that terminates on first site which is unrelated to the current parent. Also renaming alone doesn't adress the functionality concerns ...

Since this will be the new API, I don't think we will be regressing anything here.

Not a regression in your code, but you will be causing a regression in 3rd party code by misleading control developers into using this API.

We currently have this setup:

If the chart control vendor picks up your new API then this would render the chart control inoperable and/or require our end users to have design time licenses even though they never design the chart control. So you caused a regression in the reporting library (no longer able to place user controls which use your new API).

any 3rd party designer will have to opt-in to the API as well

Sorry if I missed it, but this has not been mentioned before I believe? The suggested implementation would turn any 3rd party designer into being recognized as a design-time environment. If this is opt-in for code using the WinForms designer infrastructure I'm less worried, but this hasn't been presented in this discussion yet.

The important part is that it needs to be opt-in for the host, not opt-in for the control vendor. The control vendor just wants to "detect design time scenarios", which they usually think of as Visual Studio -like designers where you design their control. The fact that WinForms designer infrastructure is used for other purposes is rarely considered by a control vendor.

(For example we use the designer infrastructure on Desktop Framework to allow the user to arrange panels for a custom layout. This is totally unrelated to any controls placed on the panels, but if any of those controls picks up the new API we will run into problems. Of course since this scenario the host is our own code we probably can work around it, assuming we even keep using WinForms designer infrastructure when we get around to port the panel layout editor.)

We could consider passing a flag, though, which controls this behavior.

The problem is, control authors don't want to bother differentiating between how they are hosted. This actively hurts composability of WinForms custom controls.

weltkante commented 3 years ago

In other words, the new API has two failure cases

If you design the new API in a way so that the host of the WinForms designer infrastructure has to opt-in to being recognized as a designer by this new API then you can adress the false-positives which are IMHO the most important problem to avoid when going forward with this API. Note that you have to do this in a way that does not break the existing DesignMode and only affects the new API.

KlausLoeffelmann commented 3 years ago

So, why is your false positive scenario false? I think I am of a different opinion here. Per definition, such a control would be in Design mode, the entity which has sited it doesn’t matter or better shouldn’t be qualified differently Once it is sited, it’s in Design mode.

We should though, if we later feel it’s necessary, address other scenarios („e.g. was instantiated/is hosted by licence-provider-designer“) in a different (license) context.

weltkante commented 3 years ago

So, why is your false positive scenario false?

Because thats how it works currently. You are allowed to use third party controls as building blocks inside end user report designers and layout editors. If you add this API in the suggested way and anyone picks this up, this usecase regresses.

Per definition, such a control would be in Design mode

No, it shouldn't be. As per this discussion, and others, people who want to check DesignMode want to do this either to check for a design time license or to disable control functionality during design time. Both are undesirable in an end user report designer or layout editor. Most control authors won't even think of these usecases, and they shouldn't have to, composability demands that you can use controls in any context.

I repeat, you are not designing the controls themselves. You are using them as a building block to build a designable control.

Once it is sited, it’s in Design mode.

Yes, the control that is sited is in design mode, but its children shouldn't be reported as in design mode, as this API suggests. This API provides a solution to a very specific problem people have, by creating a new problem for a different scenario thats much harder to work around.

KlausLoeffelmann commented 3 years ago

Again, I am afraid I am of a different opinion here. But I added a disclaimer in the API description/rationale, that this API should not be used for License-checking purposes.

Also, please keep in mind, that if you would see edge-case problems with this API for a third-party-control vendor, then by all means: don't use it for that!

I came up with this API because of the necessity for UserControl/Control authors which I personally came across many times, but which you can also look up in tens of stack-overflow posts, namely how to react differently at runtime and at design time per the 95%-valid definition of "runtime" vs. "design time".

Even if the edge cases you describe are valid (and I really thing they are!), I think they are still edge cases, and for the gross of our target audience, having this easy way for checking this condition is an enormous help!

I also added a typical UserControl scneario for clarification in the description.

terrajobst commented 3 years ago

Video

namespace System.Windows.Forms
{
    public partial class Control
    {
        protected bool IsAncestorSiteInDesignMode { get; }
    }
}
Zheng-Li01 commented 3 years ago

Verified the with latest .NET 6.0 with 6.0.100-rc.1.21411.23-win-x64, the feature has been implemented.

Ashley-Li commented 3 years ago

Verified the issue with 6.0.100-rc.1.21424.1 build, the feature has been implemented.