Remove sealed from all AutomationPeers.

[ascholz]

Hello,

Some AutomationPeers such as the DataGridRowAutomationPeer are sealed. It would be better if these were not sealed. For example, if a specific DataGridRow is created with additional parameters, we could derive from DataGridRowAutomationPeer and then add these parameters to the new AutomationPeer. But this is not possible if they are sealed.

Greetings

Andreas

[Krudonix]

Hi,

I'm not sure if I should start a new thread or post here.

Similar to above it will also be helpful if sealed class's like CalendarDayButton would be unsealed or provide a way to override the OnCreateAutomationPeer via a property or something.

Many thanks,

[ascholz]

I don't want to be a bother, but an answer as to whether it will eventually be implemented would be nice :-)

[pchaurasia14]

@ascholz - Apologies for late reply. We will be discussing this internally and assessing the impact (if any) on the original design of AutomationPeers. I'll let you know as soon as we have all the data points.

[ascholz]

@pchaurasia14 Any News on that Topic?

[lindexi]

@ascholz I think that even if it is sealed, we can still implement it by copying logic when we need to customize it.

[ascholz]

That is not possible. There are some internal Parameters which I can't access by copying the logic.

[pchaurasia14]

So, here are some notes from our internal discussions -

1. Unsealing of cooperating classes may invite breaking behaviors when subclasses are added into mix since they may have dependencies on each other (like Datagrid * AutomationPeer).
2. Unsealing of class can also make it difficult to add more behaviors in future due to name collision possibilities. Adding a new public method to an unsealed class breaks a subclass that has already declared a method with the same name.
3. Simply unsealing of class may NOT be sufficient to make it available for extension. It also involves some amount of refactoring to provide correct extension points to potential subclasses.

Sealing of these classes may have been done to guarantee future flexibility and to direct developers away from tar pits of difficulty.

IMHO, in your case, where you need to add behavior to the current class, if may be helpful to extend the functionality of the current class and make it available for benefit of the control as a whole.

Unsealing a class and adding the support to a subclass will prevent us from adding it to the base class.

[miloush]

@pchaurasia14 Thank you for sharing your notes. From my point of view the discussion on both sides is a bit too abstract. Your arguments as it stands could be used as a blank card to any request for unsealing without much space left to make a case.

1. Could you please give an example of such an invited breaking behavior?

2. This argument could be used against adding any new class in the framework, yet we will (hopefully) keep adding new ones. Member name collisions do not even appear at runtime, the derived classes would behave as if they were declared with the new keyword, i.e. newly added members to the base class wouldn't change the current program behavior (unless reflection is involved EDIT: or a colliding member is added to the base as virtual). The collision only appears at a compile time, and as a warning. That sounds like a good way to communicate to the implementors that perhaps what they implemented is now part of the framework and they no longer need to have custom, potentially different behavior. On the contrary, forcing separate classes to achieve the same makes developers oblivious to such improvements.

3. This point could be a valid point against investing in unsealing in the case of automation peers, but again, it would be great if you could include an example of why it may not be sufficient. Because the counter argument is "it may be" and there will be no progress until one side demonstrates otherwise. It is a valid request to put this burden on those who ask for it, but without voicing a specific concern, it is impossible to address it.

direct developers away from tar pits of difficulty

While it is always nice to avoid creating traps that developers can fall into, making something impossible is a too high price for it. No matter how simple or difficult something is, you will always have developers who will struggle and developers who will do the right thing. By making something impossible, you take the opportunity to learn from the less experienced and make work incredibly difficult and potentially unsupported for those who can manage.

Unsealing a class and adding the support to a subclass will prevent us from adding it to the base class.

Unless this is the same argument as 2., how?

That said, I haven't seen a convincing argument from the other side either. Could @ascholz or others be a bit more specific, can you give concrete examples of what you would like to add that you currently cannot have?

[ascholz]

Hey @miloush

thanks for the contribution :-) Here is an example:

We have added a bool parameter to a DataGridRow. A style was bound with the parameter and is on or off depending on the bool value. We now want to look via UI Automation if the style is active or not. As I understood it, we have to add the parameter to the DataGridRowAutomationPeer to check this via UI Automation because otherwise we can't check if the style is active or not. If there is another way to check this I am open for it :-)

[pchaurasia14]

@miloush - Valid points.

1. The examples that we could fathom are dealing with cases involving inspecting runtime metadata of classes. So, they MAY be a probable candidate for breaking. While it may be rare, we don't know the prevalence of such conditions in the wild.
2. Absolutely right. The argument presented was not intended to deter efforts to add new classes to the framework.
3. Simply unsealing the class will not be as useful as having hooks that allow you to customize the behavior of the class. For instance, it may help if DataGrid*AutomationPeers can expose more virtual methods that allow you to override their default behavior.

That being said, we are still open to making it unsealed.

@ascholz - We'll have to check if there is a way to cater your use case.

[ascholz]

Any news on the topic?

[cbra-caa]

Reviving this issue since we have a couple of examples where the sealed keyword makes it hard to work with the datagrid.

1. When loading data, we lock the datagrid and would like to inform screenreaders of this change - this is a custom property we have on top of the datagrid and as such we would expect to be able to extend the corresponding automationpeer with the same information.
2. The current implementation of peers for DataGridCell returns the .ToString() value of the row if the cell is empty. We have a lot of columns representing tri-state-booleans and enums, where the empty value is represented, as empty - and there we would like to override the GetNameCore() to avoid this behavior.
3. We have overriden every peer for the other controls we use - to get different screen readers to perform as we want, or inject further information like shortcuts or helpful tips - so it seems counterintuitive that we cant do this for the datagrid.

[cbra-caa]

@miloush Is there any news on this issue?

There is no viable path to changing the automation peers while they are sealed. Creating new Peer classes for Cell and Rows are not an option either since all related Peers utilize DataGridCellItemAutomationPeer.OwningPeer which uses "as DataGridCellAutomationPeer" to get the wrapper peer from the cell which will never be true due to it being a different inheritance heirarchy.

As such the only way to get DataGridCellItemAutomationPeer.GetNameCore() to not write out a debug message every time a cell is empty (not very desirable in an editable grid) is to reimplement every single peer class and their related utility.