unoplatform / uno.toolkit.ui

A set of custom controls for the WinUI and the Uno Platform not offered out of the box by WinUI, such as Card, TabBar, NavigationBar, etc.
https://platform.uno/
MIT License
83 stars 27 forks source link

[SPEC] CarouselView (FlipView + PipsPager) #360

Closed pictos closed 1 year ago

pictos commented 2 years ago

Proposal: [CarouselView]

Summary

Using the Attached Property

Rationale

You can see the POC here

Scope Attached Property

This proposal will allow devs to link the PipsPager control to a FlipView, but we can extend that to be any UIElement that has a collection (a.k.a. ItemsSource property).

Here's the spec API:

partial class SelectorExtensions : DependencyObject
{
    public static readonly DependencyProperty PipsPagerProperty;   // Attached prop.

    // Property changed method to setup the views
    static void OnDataChanged(DependencyObject dependencyObject, DependencyPropertyChangedEventArgs args);

    public static void SetPipsPager(Selector element, PipsPager value);

    public static PipsPager GetPipsPager(Selector element);
} 

partial class FlipViewExtensions: DependencyObject
{
   public static readonly DependencyProperty NextProperty;
   public static readonly DependencyProperty PreviousProperty;

   public static void SetNext(ButtonBase element, FlipView value);
   public static FlipView GetNext(ButtonBase element);

   public static void SetPrevious(ButtonBase element, FlipView value);
   public static FlipView GetPrevious(ButtonBase element);
}

Usage in the code

<Button
    x:Name="btn"
    local:FliperViewExtensions.Next="{x:Bind flipView}"
    Content="Hello" />

<FlipView
    x:Name="flipView"
    Height="180"
    MaxWidth="100"
    Margin="0,0,0,0"
    local:SelectorExtensions.PipsPager="{x:Bind pipsPager2}"
    Background="Green"
    BorderBrush="Black"
    BorderThickness="1">
    <FlipView.Items>
        <Grid>
            <Button local:FliperViewExtensions.Next="{x:Bind flipView}" Content="Next" />
        </Grid>
    </FlipView.Items>
</FlipView>
<PipsPager
    x:Name="pipsPager2"
    MinHeight="100"
    Margin="0,0,0,0"
    HorizontalAlignment="Center" />

Open Questions

~1. How will we support the usage of our Extension from DataTemplates?~

Next Step

pictos commented 2 years ago

cc: @francoistanguay @kazo0

kazo0 commented 2 years ago

@pictos We have something very similar right now in Toolkit that syncs up a TabBar to a FlipView and allows either control to update the currently selected tab bar item / currently display view in the FlipView. We can most probably work with that and re-purpose it to maybe be able to handled both of these scenarios.

https://github.com/unoplatform/uno.toolkit.ui/blob/main/src/Uno.Toolkit.UI/Behaviors/TabBarSelectorBehavior.cs

We can sync together to look over it whenever!

kazo0 commented 2 years ago

And I would lean more toward the attached property method since we have that TabBarSelectorBehavior already and it also looks to be the recommended route for PipsPage based on some of the docs: https://learn.microsoft.com/en-us/windows/apps/design/controls/pipspager#integrate-pipspager-with-a-collection-control

Recommendations

  • Common UI patterns for a PipsPager include photo viewers, app lists, carousels, and layouts where display space is limited.
  • For experiences optimized for gamepad input, we recommend against placing UI directly to the left or right of a horizontal PipsPager, and above or below a vertically oriented PipsPager.
  • For experiences optimized for touch input, we recommend integrating the PipsPager with a view control, such as a FlipView, to take advantage of on-content pagination with touch (the user can also use touch to select individual pips).

pictos commented 2 years ago

@kazo0 great, I sent you a message on teams, maybe we can discuss this on a call and I can add the conclusions here later

kazo0 commented 2 years ago

Actually, unless we want this behavior available for any type of Selector this doesn't really need a separate attached property. You can just two-way bind your PipsPager SelectedIndex to the FlipView's SelectedIndex

kazo0 commented 2 years ago

Ok, so after more discussion, there may be a benefit to synchronizing a PipsPager to a FlipView through an attached property

example:

<StackPanel>
    <FlipView x:Name="Gallery"
                Height="270"
                MaxWidth="400"
                utu:FlipViewExtensions.ContextIndicator="{Binding ElementName=FlipViewPipsPager}"
                ItemsSource="{Binding Pictures}">
        <FlipView.ItemTemplate>
            <DataTemplate x:DataType="x:String">
                <Image Source="{Binding Image}" />
            </DataTemplate>
        </FlipView.ItemTemplate>
    </FlipView>

    <PipsPager x:Name="FlipViewPipsPager"
                Margin="0,10,0,0"
                HorizontalAlignment="Center" />
</StackPanel>
kazo0 commented 2 years ago

As for the ability to have a button explicitly move forward/backward through a FlipView: This could also live in the FlipViewExtensions and we can create something like:

<FlipView x:Name="Gallery"
        Height="270"
        MaxWidth="400"
        ItemsSource="{Binding Pictures}">
    <FlipView.ItemTemplate>
        <DataTemplate>
            <StackPanel>
                <Image Source="{Binding Image}" />
                <Button utu:FlipViewExtensions.NextButton="{Binding ElementName=Gallery}"
                    Content="Next" />
            </StackPanel>
        </DataTemplate>
    </FlipView.ItemTemplate>
</FlipView>

Not sure about the naming, sort of implies we are a Button as the value for the property. NextButtonTarget? NextButtonSource?

cc @francoistanguay @Xiaoy312

kazo0 commented 2 years ago

Actually in the above Button example, would that even work if we are trying to bind to the FlipView while inside its ItemTemplate?

pictos commented 2 years ago

Actually in the above Button example, would that even work if we are trying to bind to the FlipView while inside its ItemTemplate?

@kazo0 I would say, yes... It's normal to have the button to go next and back outside the FlipView, like at the bottom.

pictos commented 2 years ago

As for the ability to have a button explicitly move forward/backward through a FlipView: This could also live in the FlipViewExtensions and we can create something like:

<FlipView x:Name="Gallery"
      Height="270"
      MaxWidth="400"
      ItemsSource="{Binding Pictures}">
  <FlipView.ItemTemplate>
      <DataTemplate>
          <StackPanel>
              <Image Source="{Binding Image}" />
              <Button utu:FlipViewExtensions.NextButton="{Binding ElementName=Gallery}"
                  Content="Next" />
          </StackPanel>
      </DataTemplate>
  </FlipView.ItemTemplate>
</FlipView>

Not sure about the naming, sort of implies we are a Button as the value for the property. NextButtonTarget? NextButtonSource?

cc @francoistanguay @Xiaoy312

I would say to do everything around FlipView, so that way the dev will know FlipView should always be passed, avoiding confusion. Like, "here should I pass the flipView or the PipsPager?". It will be something like this:

<StackPanel>
    <FlipView x:Name="Gallery"
                Height="270"
                MaxWidth="400"
                ItemsSource="{Binding Pictures}">
        <FlipView.ItemTemplate>
            <DataTemplate x:DataType="x:String">
                <Image Source="{Binding Image}" />
            </DataTemplate>
        </FlipView.ItemTemplate>
    </FlipView>

    <PipsPager x:Name="FlipViewPipsPager"
                Margin="0,10,0,0"
    ->  utu:FlipViewExtensions.FlipOwner="{Binding ElementName=Gallery}"
                HorizontalAlignment="Center" />
</StackPanel>
Xiaoy312 commented 2 years ago

re: https://github.com/unoplatform/uno.toolkit.ui/issues/360#issuecomment-1276597105

you shouldnt be including buttons in the ItemTemplate; flipview also has them built-in and you can use its VisualStates to properly toggle visibility/enable-ness of prev/next buttons (so you dont have to deal with the logic of "show next if not last") ElementName binding across data-template seems like a recipe for disaster (idk if uwp/uno supports this) these are more directed to your sample; the idea is fine

re: https://github.com/unoplatform/uno.toolkit.ui/issues/360#issuecomment-1274846022 they arent many descendants of Selector that would fit here:

- X ComboBox
+ O FlipView
- X ListBox
- X ListViewBase:
-   - X GridView
-   - X ListView

in fact, only FlipView seem like the sane choice

Xiaoy312 commented 2 years ago

re: https://github.com/unoplatform/uno.toolkit.ui/issues/360#issuecomment-1276644612 it wouldn't be a problem with documentation (md doc + XML documentation on property+accessor) and proper naming eg: utu:FlipViewExtensions.AttachedPipsPager=... not hard to guess which goes where

francoistanguay commented 2 years ago

Ok some extra thoughts for discussion:

First, if Items Count/SelectedIndex to be 2 way databound is on Selector and not FlipView, we should move the AP to SeleftorExtensions.Indicator or SelectorExtensions.Pager (as it's specific to PisPager).

I understand why setting one AP instead of 2 bindings is easier and could be fine with it.

As for Next/Previous AP, it's not the same thing. In the context of a Wizard, FlipView would be used with explicit pages/content, not with ItemTemplate + a DataSource... we're talking Wizard here, not Photo Viewer... So we do need explicit buttons on a per page basis, not a style update.

pictos commented 2 years ago

FYI I updated the spec.

francoistanguay commented 2 years ago

I thought FlipViewExtensions Next/Previous should accept ButtonBase and not Button?

And SelectorExtensions should extend a Selector, not a FlipView

pictos commented 2 years ago

Ops, my bad. Fixed

francoistanguay commented 2 years ago

LGTM