Skip to content

Change parts of Expander implementation to support animation #2522

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

stephenquan
Copy link
Contributor

@stephenquan stephenquan commented Feb 14, 2025

Description of Change

Refactor Expander to support animations:

  • bool AnimationsEnabled { get; set; } // default: true;
  • Easing? CollapseEasing { get; set; } // default: null;
  • unit CollapsingDuration { get; set; } // default: 250u;
  • Easing? ExpandEasing { get; set; } // default: null;
  • unit ExpandDuration { get; set; } // default: 250u;

Reorganize the Expander to be implemented similar to the following XAML pseudo-code:

<Grid RowDefinitions="Auto,Auto">
    <ContentView Content="{Binding Header}"/>
    <!-- Animations on VerticalStackLayout.Height from 1 (Collapsed) to Content.Height+1 (Expanded) -->
    <VerticalStackLayout Padding="0,1,0,0" HeightRequest="1">
       <ContentView Content="{Binding Content}"/>
    </VerticalStackLayout>
</Grid>

Linked Issues

#2521

Linked Discussions

#1628

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: TODO: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

This PR was created to solicit suggestions from the team.

…t instead of Content.IsVisible so that the user can use animate expanding/collapsing modes.
@dotnet-policy-service dotnet-policy-service bot added stale The author has not responded in over 30 days help wanted This proposal has been approved and is ready to be implemented labels Mar 23, 2025
@IeuanWalker
Copy link
Contributor

Just want to add i've been using this package for my expander - https://github.com/ewerspej/epj.Expander.Maui
(have made some changes locally to improve accessibility)

Might find some use from the code/ properties they have.

@stephenquan stephenquan mentioned this pull request Apr 3, 2025
8 tasks
@stephenquan
Copy link
Contributor Author

@TheCodeTraveler @VladislavAntonyuk based on today's (April) standup I have updated the Expander implementation to have animation properties. The code is not fully quite right because the last 2 edge conditions "Expander in CollectionView with LinearItemsLayout" and "Expander in CollectionView with GridItemsLayout" aren't working right.

@IeuanWalker
Copy link
Contributor

IeuanWalker commented Apr 4, 2025

Not sure if its possible in the current implementation or not. But in the modified version of https://github.com/ewerspej/epj.Expander.Maui i added a property (HasHeader) to remove/ hide the Header

This then allows me to use the expander to expand/ collapse any content, f.e. an alert at the top of the page -

2025-04-04.18-05-02.mp4

@stephenquan
Copy link
Contributor Author

@IeuanWalker I'm not sure what your requirements in, but couldn't it be covered by setting Direction=Up and/or making the Header hidden/invisible?

@IeuanWalker
Copy link
Contributor

IeuanWalker commented Apr 7, 2025

@stephenquan sorry havnt really got time to dig into the code much.

Main issue with the other one was that a ContentView with a TapGesture was being added as a header automatically. So i wasnt setting the HeaderContent, and visually it looked completely fine. (no visible header like in the video)

But accessability wise the screenreader/ keyboard users could still interact with the blank ContentView/ TapGesture.

So added the new property to bind to the IsVisible property of the ContentView (which previously i couldn't influence outside of the control) to prevent the ContentView/ TapGesture from being accessed by the accessibility tools

@VladislavAntonyuk
Copy link
Collaborator

Thank you for your PR @stephenquan . As we discussed during the Community toolkit standup, it would be better to have a single bindable property, where the user can set AnimationBehavior he wants. It would be nice if we could reuse .NET MAUI Community Toolkit animations. Would you like to implement such behavior?

@stephenquan
Copy link
Contributor Author

Thanks @VladislavAntonyuk, I don't think I fully understand, but I would like to. So, I will start by researching how the other CommunityToolkit Behaviors are written - is there any specific sample, code snippet I should be paying attention to?

@VladislavAntonyuk
Copy link
Collaborator

@stephenquan I mean instead of custom implementation and adding all properties like duration etc, create a single property and allow users set the whole animation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted This proposal has been approved and is ready to be implemented stale The author has not responded in over 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants