dotnet / winforms

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

FontNameEditor cannot be registered #2104

Open weltkante opened 5 years ago

weltkante commented 5 years ago

In PR #2103 following comment was noticed:

no way to add Font.Name and associate it with FontNameEditor

Thats unfortunate, but the good news is that its the only [Editor] attribute in System.Drawing thats placed on a property (verified by doing reflection on the Desktop Framework assembly, looping over all types and properties)

Considering that the only purpose of this "editor" is to draw the font name in the same style as the font it's not even an editor. I want to open a discussion on how to proceed with this:

a1) remove FontNameEditor entirely since it provides no actual value to the user editing

a2) keep FontNameEditor (since it is public API and someone may happen to use it) but do not link it to the Font.Name property. This would basically keep the behavior as it is now (which does not match Desktop Framework) and make it "official" for .NET Core. Third party users happening to use the editor would still compile.

b1) remove FontNameEditor from WinForms and place it in the VS designer instead. The VS Designer Package already has its own more complex extensions to TypeDescriptor and might have less trouble adding this one. This means for user-written designers it won't be available.

b2) similar to a2, keep FontNameEditor in WinForms since its public API, but like described in b1 only link it up in the VS designer. Third party users outside the VS designer could use it but would need to register it manually.

c) extend TypeDescriptor in CoreFX to allow registering default editors on properties, keep the FontNameEditor in WinForms and register it using the new functionality

Given this doesn't impact any functionality (its just for visuals) it probably doesn't need to go into 3.1 and can be resolved later.

zsd4yr commented 5 years ago

a) drop FontNameEditor entirely since it provides no actual value to the user editing

Can you clarify what you mean by this?

zsd4yr commented 5 years ago

Funny enough, we have been discussing this internally for some time! I do not think we have come to a final solution yet

weltkante commented 5 years ago

a) drop FontNameEditor entirely since it provides no actual value to the user editing

Can you clarify what you mean by this?

This option would just be removing the editor subclass without a replacement, the editor subclass just slightly changes how the value is displayed, it is only visual and provides no additional inputing capabilities nor assist the user in any way editing the value.

I'm not saying its the best option, but its certainly one you could argue for if the others are not feasible.

Considering the class is public API there would also be variant of not removing the class entirely, just not linking it to the Font.Name property anymore (basically the way it is now) - allowing external dependencies compile without error if they should happen to use it.

In the original issue description I didn't notice the editor was public, I extended the options (a) and (b) with variants not removing the public API, just keeping it nonfunctional. (I now realize original (a1) and (b1) are breaking and most likely not being worth discussing, can remove them if you prefer that.)