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

Switch SplitView.IsPaneOpen to a StyledProperty #10138

Merged
merged 11 commits into from
Feb 22, 2023

Conversation

robloo
Copy link
Contributor

@robloo robloo commented Jan 31, 2023

What does the pull request do?

Switches the SplitView.IsPaneOpen property to a StyledProperty. This follows the Expander control changes (SplitView was originally based on Expander for this property #9944 (comment)).

cc @amwx (original author)

What is the current behavior?

IsPaneOpen is a DirectProperty and cannot be styled.

What is the updated/expected behavior with this PR?

  1. IsPaneOpen is now a full StyledProperty and usable in the styling system.
  2. Modernized some code to follow latest conventions
    • Removed sender parameter from OnClosing/OnOpening/etc. protected virtual methods. These are not usually provided and are redundant (OnPropertyChanged itself is an example).
    • Reformatted some property definitions to span multiple lines and reduce code width
    • Use OnPropertyChanged for all property change handling
    • Moved all SplitView types into their own directory (also now uses one file for each type)
  3. Upgraded SplitView events to be fully routed events. Removes custom SplitViewPaneClosingEventArgs so this now differs from UWP but matches the Expander and more general patterns.
  4. Support cancellation from both PaneOpening/PaneClosing events (following the Expander pattern)
  5. Fixed a mistake in Expander using RoutedEventArgs instead of CancelRoutedEventArgs in OnCollapsing/OnExpanding methods.

How was the solution implemented (if it's not obvious)?

See code.

Checklist

Breaking changes

Yes

  • StyledProperty instead of DirectProperty but functionality is largely only expanded for app developers
  • RoutedEvents instead of CLR events
  • Removed SplitViewPaneClosingEventArgs in favor of CancelRoutedEventArgs
  • Supporting cancellation in PaneOpening so args changes

Obsoletions / Deprecations

None

Fixed issues

Fixes #9800
Part of #9944

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0029299-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@robloo
Copy link
Contributor Author

robloo commented Feb 19, 2023

@amwx For the PaneClosing event you made it a CLR (non-routed) event. I'm assuming that was for API compatibility with UWP. You also copied over the SplitViewPaneClosingEventArgs which supports cancellation instead of using CancelEventArgs already built-in to .NET.

@tomenscape In your StyledProperty branch you switched this control to using routed events and this would switch to CancelRoutedEventArgs (a breaking change). And also support cancellation in opening.

Is everyone OK with using @tomenscape approach? This standardizes with Expander:

  1. Breaking change removing the SplitViewPaneClosingEventArgs and using the CancelRoutedEventArgs instead
  2. Binary breaking change using routed events instead of CLR events
  3. Breaking change to support cancellation in the Opening event as well.

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0030251-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@robloo robloo marked this pull request as ready for review February 19, 2023 02:31
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0030256-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member

These breaking changes don't seem critical and are quite easy to fix in the user code. Keeping in mind we are in preview stage still.

@grokys grokys enabled auto-merge February 22, 2023 16:16
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0030416-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0030436-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@grokys grokys merged commit 80394d0 into AvaloniaUI:master Feb 22, 2023
@robloo robloo deleted the SplitView-StyledProperty branch February 23, 2023 00:20
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

Successfully merging this pull request may close these issues.

Cannot set direct property 'IsPaneOpen' in 'SplitView' because the style has an activator.
4 participants