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

GetChildrenInTabFocusOrder no longer a member of the generated code #1395

Closed
martgoodall opened this issue Mar 7, 2024 · 10 comments
Closed

Comments

@martgoodall
Copy link

Version

2.0.240111.5

Summary

after upgrading from 2.0.230706.1 to 2.0.240111.5 , GetChildrenInTabFocusOrder() is no longer a member of the generated code, so the winappsdk/winrt c++ app no longer compiles the exe.

WinUI.zip

Reproducible example

pre upgrade, the following would compile - see attachment for complete source containing the following code:-

c++
   StackPanel stackPanelItems = containerContentChangingEventArgs.ItemContainer().ContentTemplateRoot().as<StackPanel>();
   auto autoItems = stackPanelItems.GetChildrenInTabFocusOrder();

GetChildrenInTabFocusOrder is not generated as a member of the class.

Expected behavior

to still compile existing code under the new release

Actual behavior

GetChildrenInTabFocusOrder is not generated as a member of the class.

Additional comments

app is built successfully with the following:-

app fails to build by upgrading
Microsoft.Windows.CppWinRT

@dmachaj
Copy link
Contributor

dmachaj commented Mar 7, 2024

@sylveon this appears to be another question related to #1319.

If I understand the description of that PR it should be considered invalid for something outside the StackPanel implementation to call GetChildrenInTabFocusOrder.

@sylveon
Copy link
Contributor

sylveon commented Mar 7, 2024

It's an overridable method on UIElement, which means it is protected. So yeah, the proper way would be to inherit from StackPanel to use it, like one would in C#. Alternatively as a workaround, using sp.as<IUIElementOverridable>().GetChildrenInTabFocusOrder() should work (I may have the exact name of the interface wrong).

@martgoodall
Copy link
Author

I really appreciate the feedback.
I was able to get it to compile under 1.5.0 with the following change:-

auto autoItems = stackPanelItems.as().GetChildrenInTabFocusOrder();
(which is almost what you suggested but IUIElementOverrides instead of IUIElementOverridable).

using winrt c++/windowsappsdk/Xaml, I'm really not sure how to implement the other suggestion of "inherited StackPanel in a similar fashion to the C#".....if anyone had a code sample doing similar, it would be very appreciated, since there are so few decent winrt c++/windowsappsdk/Xaml examples on the web.

again many thanks for the feedback.

@sylveon
Copy link
Contributor

sylveon commented Mar 9, 2024

It is not very obvious, but the way to do this is to:

  • Create a blank page from VS (right-click the project in the Solution Explorer > Add > New Item... > Blank Page (WinUI 3))
  • Open the IDL, change Microsoft.UI.Xaml.Controls.Page to Microsoft.UI.Xaml.Controls.StackPanel
  • Open the XAML, change <Page to <StackPanel and </Page> to </StackPanel>
  • Put some contents, or if you don't want to add XAML to that class and delegate setting the contents to consumers, you can use the View Model (C++/WinRT) template when creating a new item instead of Blank Page (WinUI 3)
  • You should now be able to access GetChildrenInTabFocusOrder within the class:
    image

@torleifat
Copy link

torleifat commented Mar 11, 2024

So I stumbled across this issue just recently. However I also discovered another issue which I think is related, and even if it is, it might be outside the scope of cppwinrt to fix(if it should be fixed at all).

Attempting to call an overridable function from a derived class in Xaml

Say you have an IDL like this:

    [default_interface]
    unsealed runtimeclass A
    {
        A();

        overridable void MyFancyFunction();
    }

    [default_interface]
    runtimeclass B : A
    {
        B();
    }

    [default_interface]
    runtimeclass MainWindow : Microsoft.UI.Xaml.Window
    {
        MainWindow();

        B Model{ get; };
    }

And in your MainWindow xaml you call on MyFancyFunction, something like this:

<Button x:Name="myButton" Click="{x:Bind Model.MyFancyFunction}">Click Me</Button>

In 2.0.230706.1 this will compile and work fine. In 2.0.240111.5 it will lead to a compile-error where it could not find the MyFancyFunction as a member of B.
If this was in code I could do something like Model().as<IAOverrides>().MyFancyFunction(). However seeing as this is done in Xaml it complicates matters. Note that there's ways around this, by adding MyFancyFunction to the B's IDL, and not adding any actual override in the implementation it'll successfully call A's MyFancyFunction. In this specific case we could also just remove the override flag, but in a theoretical case there could be a derived class C, which needs to override MyFancyFunction.

Now there's a chance that there are good reasons why this should not be allowed to begin with. I am not very well versed in MIDL 3.0 or C++/Winrt, but I am wondering if this intended. For me it does seem a little counter-intuitive that this should not work.

@sylveon
Copy link
Contributor

sylveon commented Mar 11, 2024

This is indeed intended. To quote the WinRT type system:

A composable class may declare zero or more of its member interfaces to be overridable. An overridable interface may only be queried for, and used within, a composition chain—similar to the rules about accessing protected interfaces detailed previously. However, where a protected interface may only be implemented by the class that originally declared it, overridable interfaces may be re-implemented by classes that compose the class that implemented the overridable interface.

@martgoodall
Copy link
Author

martgoodall commented Mar 11, 2024

Thanks for the suggestion.

I believe the new attachment is what you suggested and appears to work but could you please confirm it looks right to you as this stuff is all new to me and it seems to me to be a strange change to now protect GetChildrenInTabFocusOrder forcing a custom method requirement for basic functionality.

WinUI.zip

Xaml reads as follows ( changed to local:StackPanelViewModel) :-

============================

 <local:StackPanelViewModel 
             x:Name="StackPanelQueue"
             AllowDrop="True"
             DragOver="GridViewQueues_DragOver"
             Drop="GridViewQueues_Drop"
             Width="200"
             Height="220">

========================

the c++ file reads as follows :-

// old code
// StackPanel stackPanelItems = containerContentChangingEventArgs.ItemContainer().ContentTemplateRoot().as();
//
// old code
// for (auto autoStackPanelItem : stackPanelItems.as().GetChildrenInTabFocusOrder())
// {
// }

// new code
auto autoStackPanelViewModel = containerContentChangingEventArgs.ItemContainer().ContentTemplateRoot().try_as();
if (autoStackPanelViewModel == nullptr)
co_return;

for (auto autoStackPanelItem : autoStackPanelViewModel.get()->ViewModelGetChildrenInTabFocusOrder())
{
}

===================

thanks
Mart

It is not very obvious, but the way to do this is to:

  • Create a blank page from VS (right-click the project in the Solution Explorer > Add > New Item... > Blank Page (WinUI 3))
  • Open the IDL, change Microsoft.UI.Xaml.Controls.Page to Microsoft.UI.Xaml.Controls.StackPanel
  • Open the XAML, change <Page to <StackPanel and </Page> to </StackPanel>
  • Put some contents, or if you don't want to add XAML to that class and delegate setting the contents to consumers, you can use the View Model (C++/WinRT) template when creating a new item instead of Blank Page (WinUI 3)
  • You should now be able to access GetChildrenInTabFocusOrder within the class:
    image

@sylveon
Copy link
Contributor

sylveon commented Mar 11, 2024

Seems fine to me - FWIW GetChildrenInTabFocusOrder has always been protected virtual in C#.

@DefaultRyan
Copy link
Member

Say you have an IDL like this:

    [default_interface]
    unsealed runtimeclass A
    {
        A();

        overridable void MyFancyFunction();
    }

    [default_interface]
    runtimeclass B : A
    {
        B();
    }

    [default_interface]
    runtimeclass MainWindow : Microsoft.UI.Xaml.Window
    {
        MainWindow();

        B Model{ get; };
    }

And in your MainWindow xaml you call on MyFancyFunction, something like this:

<Button x:Name="myButton" Click="{x:Bind Model.MyFancyFunction}">Click Me</Button>

In 2.0.230706.1 this will compile and work fine. In 2.0.240111.5 it will lead to a compile-error where it could not find the MyFancyFunction as a member of B. If this was in code I could do something like Model().as<IAOverrides>().MyFancyFunction(). However seeing as this is done in Xaml it complicates matters. Note that there's ways around this, by adding MyFancyFunction to the B's IDL, and not adding any actual override in the implementation it'll successfully call A's MyFancyFunction. In this specific case we could also just remove the override flag, but in a theoretical case there could be a derived class C, which needs to override MyFancyFunction.

Now there's a chance that there are good reasons why this should not be allowed to begin with. I am not very well versed in MIDL 3.0 or C++/Winrt, but I am wondering if this intended. For me it does seem a little counter-intuitive that this should not work.

First, I'll note that you don't need overridable to customize the implementation within your component. You can do that by making the internal implementation method(s) virtual. You only need to mark a method overridable if your intent is to give component-external consumers the ability to customize some behavior in a way that is visible to the base type(s) in your [shipped] component, and so you needed some way of making this customizability part of the public-facing interface of your component for consumers to extend later on.

It's common practice (I'd even say "standard practice") to separate interface from implementation by making the interface public and the implementation non-public. When dealing with virtual functions, it's a common idiom (but not universal) to treat the public interface as stable and non-virtual, and to treat the customizeable/virtual behavior as implementation by making it non-public. This pattern can be especially useful when the overridable bit needs to be surrounded by pre- and post- action.

A familiar example of this is XAML's Measureand MeasureOverride. Measure is non-virtual, while subclasses can customize their behavior by overriding MeasureOverride. Only the base Measure is callable publicly, and it performs work both before and after calling the overridable MeasureOverride.

Coming back to your example, you need to decide if you truly need overridable in your IDL for external components to subclass and customize your type. You probably don't, and will be able to remove overridable from MyFancyFunction in your IDL. If you want custom behavior between A and B, just make your implementation function virtual. While you're at it, it's probably worth checking your design to verify that B truly "is-a" A - whether it's required that A and B be so tightly coupled in both interface and implementation, or if you'd be better served by separating the two and offloading this common behavior into an interface:

interface IFancyFunction
{
  void FancyFunction();
}

[default_interface]
runtimeclass A : IFancyFunction
{
  A();
}

[default_interface]
runtimeclass B : IFancyFunction
{
  B();
}

Finally, if you truly need B to derive from A, and you truly need a component-external overridable method. Declare a public non-overridable method in the base that calls the overridable method in subclasses. Here Be Dragons - the actual implementation is non-trivial and easy to get wrong. You've been warned. :)

unsealed runtimeclass A
{
  A();

  void FancyFunction();
  overridable void FancyFunctionOverride();
}

[default_interface]
runtimeclass B : A
{
  B();
}

(Fun trivia, C++ virtual function behavior can sometimes get non-intuitive in the face of hiding and overloading - https://isocpp.org/wiki/faq/strange-inheritance#protected-virtuals)

@torleifat
Copy link

Thanks for the insight @DefaultRyan.
The original code where this came up does indeed not require overridable . Since it is not really customer exposed code to begin with, so it was a quick fix, and as you say if needed an override can be handled in the implementation instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants