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

SettingsExpander in SettingsExpander crashes the application #560

Open
3 of 24 tasks
naumenkoff opened this issue Nov 16, 2024 · 4 comments
Open
3 of 24 tasks

SettingsExpander in SettingsExpander crashes the application #560

naumenkoff opened this issue Nov 16, 2024 · 4 comments
Assignees
Labels
bug Something isn't working ux 🖌️ Everything design related

Comments

@naumenkoff
Copy link

Describe the bug

If you define a SettingsExpander as an item within another SettingsExpander, the application crashes.

<controls:SettingsExpander Header="Parent">
    <controls:SettingsExpander.Items>
        <controls:SettingsExpander Header="Child" />
    </controls:SettingsExpander.Items>
</controls:SettingsExpander>

Steps to reproduce

1. Add a `SettingsExpander` to the layout.  
2. Add another `SettingsExpander` as a nested item inside the first `SettingsExpander`.  
3. Run the application.  
4. Observe that the application crashes.

Expected behavior

The application should not crash.

  • Automatically hide/not render the nested SettingsExpander.
  • Alternatively, the build process could prevent such layout from compiling/add hint.
  • Explicitly marking nested SettingsExpander usage as prohibited in the gallery documentation.

Screenshots

No response

Code Platform

  • UWP
  • WinAppSDK / WinUI 3
  • Web Assembly (WASM)
  • Android
  • iOS
  • MacOS
  • Linux / GTK

Windows Build Number

  • Windows 10 1809 (Build 17763)
  • Windows 10 1903 (Build 18362)
  • Windows 10 1909 (Build 18363)
  • Windows 10 2004 (Build 19041)
  • Windows 10 20H2 (Build 19042)
  • Windows 10 21H1 (Build 19043)
  • Windows 10 21H2 (Build 19044)
  • Windows 10 22H2 (Build 19045)
  • Windows 11 21H2 (Build 22000)
  • Other (specify)

Other Windows Build number

Windows 11 24H2 (Build 26100)

App minimum and target SDK version

  • Windows 10, version 1809 (Build 17763)
  • Windows 10, version 1903 (Build 18362)
  • Windows 10, version 1909 (Build 18363)
  • Windows 10, version 2004 (Build 19041)
  • Windows 10, version 2104 (Build 20348)
  • Windows 11, version 22H2 (Build 22000)
  • Other (specify)

Other SDK version

No response

Visual Studio Version

Preview

Visual Studio Build Number

17.12.0 Preview 5.0

Device form factor

Desktop

Additional context

CommunityToolkit.WinUI.Controls.SettingsControls: 8.1.240916

Help us help you

No, I'm unable to contribute a solution.

@michael-hawker
Copy link
Member

@niels9001 this isn't something I think we ever considered with the Settings pattern, eh?

I would guess it's related to #302 as SettingsExpander expects SettingsCards as children, and SettingsExpander itself (while looking and using a Card, isn't one itself).

Is this a special case we should allow? Or is this an odd design pattern we want to discourage?

@naumenkoff can you share the scenario for what you were trying to achieve?

@naumenkoff
Copy link
Author

naumenkoff commented Nov 20, 2024

@niels9001 this isn't something I think we ever considered with the Settings pattern, eh?

I would guess it's related to #302 as SettingsExpander expects SettingsCards as children, and SettingsExpander itself (while looking and using a Card, isn't one itself).

Is this a special case we should allow? Or is this an odd design pattern we want to discourage?

@naumenkoff can you share the scenario for what you were trying to achieve?

These are just my observations 🙃 I don't think that this is a bad thing, because everything has its own limitations and it would be good to mention it somewhere (Gallery?) (I didn't find such information)

@niels9001
Copy link
Collaborator

Yeah, this isn't a pattern I have seen before across Windows. I can imagine it might come with some accesibility issues as well when nesting different expanders?

@naumenkoff do you have a specific UI in mind with the code you provided?

@michael-hawker @Arlodotexe is there a straightforward way to throw a warning or something else when not using a SettingsCard?

@naumenkoff
Copy link
Author

naumenkoff commented Nov 20, 2024

Yeah, this isn't a pattern I have seen before across Windows. I can imagine it might come with some accesibility issues as well when nesting different expanders?

@naumenkoff do you have a specific UI in mind with the code you provided?

@michael-hawker @Arlodotexe is there a straightforward way to throw a warning or something else when not using a SettingsCard?

No, sorry, I don’t have a UI in mind with this approach because I don’t like deep nesting. I actually stumbled upon this completely by accident during a refactor when I wanted to group several SettingsCards (the first being a ToggleSwitch and the second a TextBox) under a single SettingsExpander and accidentally replaced them both with a SettingsExpander.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ux 🖌️ Everything design related
Projects
None yet
Development

No branches or pull requests

4 participants