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

ListDetailsView not working with WinUI3/WindowsAppSDK 1.0.0-preview3 #4357

Closed
6 of 21 tasks
huoyaoyuan opened this issue Nov 1, 2021 · 13 comments · Fixed by #4738
Closed
6 of 21 tasks

ListDetailsView not working with WinUI3/WindowsAppSDK 1.0.0-preview3 #4357

huoyaoyuan opened this issue Nov 1, 2021 · 13 comments · Fixed by #4738
Assignees
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 controls 🎛️ WinUI 💠 Related to WinUI 3 Version or when paired with External can mean requires fix in WinUI 2/3.
Milestone

Comments

@huoyaoyuan
Copy link
Contributor

huoyaoyuan commented Nov 1, 2021

Describe the bug

ListDetailsView causes hard crash in normal usage.

  • Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work:
    7.0.1

Steps to Reproduce

  • Can this be reproduced in the Sample App? (Either in a sample as-is or with new XAML pasted in the editor.) If so, please provide custom XAML or steps to reproduce. If not, let us know why it can't be reproduced (e.g. more complex setup, environment, dependencies, etc...)

Steps to reproduce the behavior:

  1. Create a WinUI3/WAS project, install toolkit 7.1.1-preview3
  2. Add a ListDetailsView, set ListPaneWidth and some content
  3. if ListPaneWidth set in xaml, it will fail with XamlParseException which wrappes NullReferenceException
    4. When moving mouse over the control. it will fail with not being able to load reveal brush
    Edit: I used reveal brush in my custom style. It's not caused by the control itself.

Environment

NuGet Package(s):
CommunityToolkit.WinUI.UI.Controls.Layout
Microsoft.WindowsAppSDK 1.0.0-preview3

Package Version(s):
7.1.1-preview3

Windows 10 Build Number:

  • Fall Creators Update (16299)
  • April 2018 Update (17134)
  • October 2018 Update (17763)
  • May 2019 Update (18362)
  • May 2020 Update (19043)
  • Insider Build ({build_number})

App min and target version:

  • Fall Creators Update (16299)
  • April 2018 Update (17134)
  • October 2018 Update (17763)
  • May 2019 Update (18362)
  • May 2020 Update (19041)
  • Insider Build ({build_number})

Device form factor:

  • Desktop
  • Xbox
  • Surface Hub
  • IoT

Visual Studio version:

  • 2017 (15.{minor_version})
  • 2019 (16.{minor_version})
  • 2022 (17.{minor_version})

Additional context

This could be a issue of WinUI side, but we should be aware of them here.

@huoyaoyuan huoyaoyuan added the bug 🐛 An unexpected issue that highlights incorrect behavior label Nov 1, 2021
@ghost ghost added the needs triage 🔍 label Nov 1, 2021
@ghost
Copy link

ghost commented Nov 1, 2021

Hello huoyaoyuan, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@huoyaoyuan huoyaoyuan changed the title ListDetailsView broken with WinUI3/WindowsAppSDK 1.0.0-preview3 ListDetailsView.ListPaneWidth broken with WinUI3/WindowsAppSDK 1.0.0-preview3 Nov 3, 2021
@huoyaoyuan huoyaoyuan changed the title ListDetailsView.ListPaneWidth broken with WinUI3/WindowsAppSDK 1.0.0-preview3 ListDetailsView not working with WinUI3/WindowsAppSDK 1.0.0-preview3 Nov 3, 2021
@huoyaoyuan
Copy link
Contributor Author

The call stack is native SystemNavigationManager::GetForCurrentView, so this is microsoft/microsoft-ui-xaml#4174 again.

@michael-hawker
Copy link
Member

michael-hawker commented Nov 4, 2021

Thanks @huoyaoyuan I think we missed from the original WinUI discussion that there was code we needed to update here too. Thanks for filing an issue.

@azchohfi looks like we missed how best to convert these APIs here:

if (Window.Current != null)
{
SystemNavigationManager.GetForCurrentView().BackRequested += OnBackRequested;
}

if (Window.Current != null)
{
SystemNavigationManager.GetForCurrentView().BackRequested -= OnBackRequested;
}

According to the docs this isn't a WinUI 3 API at all. We used it here to synchronize the behavior of the ListDetailsView to the main app back button. Since that concept doesn't exist, I think we just remove this code entirely in our WinUI 3 world, right?

I think our original hope was that Window.Current was going to be a guard for this, or am I mis-remembering?

@michael-hawker michael-hawker added controls 🎛️ WinUI 💠 Related to WinUI 3 Version or when paired with External can mean requires fix in WinUI 2/3. and removed needs triage 🔍 labels Nov 4, 2021
@michael-hawker michael-hawker added this to the WinUI3 milestone Nov 4, 2021
@huoyaoyuan
Copy link
Contributor Author

Since that concept doesn't exist, I think we just remove this code entirely in our WinUI 3 world, right?

How about WinUI 3 with UWP? On Win32 I think we can remove it.

@michael-hawker
Copy link
Member

@huoyaoyuan sorry are you creating a WinUI 3 UWP project? We only support WinUI 3 Desktop projects with the Toolkit.

@michael-hawker
Copy link
Member

@huoyaoyuan
Copy link
Contributor Author

@michael-hawker No, just in case if you are missing that scenario.

@michael-hawker
Copy link
Member

@huoyaoyuan we just shipped an update which we think will address the issue?https://github.com/CommunityToolkit/WindowsCommunityToolkit/releases/tag/winui-7.1.1-preview3.1

Please let us know, thanks!

@huoyaoyuan
Copy link
Contributor Author

@michael-hawker Yes it works.

However, I discovered that ListPaneWidth has a another issue. It throws NRE (0x80004003 : 'Object reference not set to an instance of an object.') if I set it in xaml. It works correctly if I set it later in xaml hot reload.

This is not so critical and I can wait for next version.

@suchja
Copy link

suchja commented Nov 18, 2021

I do have the same problem with ListPaneWidth. Unfortunately it comes out of some template and I can't find any workaround for this:

Bildschirmfoto 2021-11-18 um 17 49 34

As far as I can see there is no custom template I supply which changes the ListPaneWidth. So I'm currently not able to use the ListDetailView. I'm using winui-7.1.1-preview3.1 and tested with .NET 5.0 as well as .NET 6.0.

@DanyaSWorlD
Copy link

Have same isuue. Any workarounds? I'm using version 7.1.2 and .NET 6.0

@michael-hawker
Copy link
Member

Was able to reproduce this in UWP as well, so not specific to WinUI 3. It was overlooked with the change to TwoPaneView for this component in PR #3768, so setting ListPaneWidth before loading is just broken due to lack of null check in OnListPaneWidthChanged, fix should be:

        private void OnListPaneWidthChanged()
        {
            if (_twoPaneView != null)
            {
                _twoPaneView.Pane1Length = new GridLength(ListPaneWidth);
            }
        }

@LalithaNadimpalli
Copy link
Contributor

Hi, Thank you for raising the Issue, Created a PR with the suggested changes

Repository owner moved this from In Progress to Done in Windows Community Toolkit Sep 2, 2022
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Sep 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥 controls 🎛️ WinUI 💠 Related to WinUI 3 Version or when paired with External can mean requires fix in WinUI 2/3.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants