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

Add sample page for LayoutTransformControl #1917

Merged
merged 8 commits into from Apr 23, 2018
Merged

Add sample page for LayoutTransformControl #1917

merged 8 commits into from Apr 23, 2018

Conversation

Odonno
Copy link
Contributor

@Odonno Odonno commented Mar 22, 2018

Issue: #1817

PR Type

  • Sample app changes

What is the current behavior?

No sample page for the LayoutTransformControl.

What is the new behavior?

A sample page for the LayoutTransformControl.

Other information

I added the sample page with supported Transform operations (Rotate, Scale, Skew).

I still need to add an image.

@nmetulev I also noticed there is no Markdown documentation for it. I think we should do it. Maybe another PR?

@skendrot
Copy link
Contributor

To show that's it's doing more than just animating the object, surround it with elements like border or rectangle to show how items around it change as well

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think main thing is just missing an icon, but expanded example could be useful. Maybe found a separate bug, not sure, thoughts?

"About": "Control that implements support for transformations as if applied by LayoutTransform.",
"CodeUrl": "https://github.com/Microsoft/UWPCommunityToolkit/tree/master/Microsoft.Toolkit.Uwp.UI.Controls/LayoutTransformControl",
"XamlCodeFile": "LayoutTransformControlXaml.bind",
"Icon": "/SamplePages/LayoutTransformControl/LayoutTransformControl.png"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Icon missing from PR?

<SkewTransform AngleX="@[SkewX:DoubleSlider:0:-180.0-180.0]"
AngleY="@[SkewY:DoubleSlider:0:-180.0-180.0]" />
</TransformGroup>
</controls:LayoutTransformControl.Transform>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @skendrot, could be useful to show more of what it's doing compared to a RenderTransformOrigin.

I did notice that with similar values in Skew though that at some point the layout transform seems to start behaving differently...

image

    <Grid Background="{StaticResource Brush-Grey-01}" Grid.Row="1"
          HorizontalAlignment="Center" VerticalAlignment="Center"
          RenderTransformOrigin="0.5,0.5">
        <Grid.RenderTransform>
          <TransformGroup>
            <RotateTransform Angle="-2.25" />
            <ScaleTransform ScaleX="2.21"
                          ScaleY="2.2" />
            <SkewTransform AngleX="-120"
                           AngleY="180" />
          </TransformGroup>
        </Grid.RenderTransform>
      <Grid HorizontalAlignment="Center" VerticalAlignment="Center" Margin="10">
        <TextBlock Foreground="White" Text="This is a test message." />
      </Grid>
    </Grid>

Compared to when there are less large values:

image

Would this be a bug somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. Your example is much more complicated than the code you gave. One thing I can notice is that in your large example, the first rendered grid is correct but not the second one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Odonno both images are the same example, only difference is the change in Skew Angles (-20, 20) for the bottom and (-120, 180) for the top. The top rectangle in each image is the Layout Transform and the bottom rectangle is the Render Transform. I just added it into the example as another row of the main grid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have achieved to replicate the second render and here is the configuration I set:

<controls:LayoutTransformControl Background="{StaticResource Brush-Grey-01}"
                                     HorizontalAlignment="Center"
                                     VerticalAlignment="Center"
                                     RenderTransformOrigin="0.5,0.5">
      <controls:LayoutTransformControl.Transform>
        <TransformGroup>
          <RotateTransform Angle="-2.25" />
          <ScaleTransform ScaleX="2.21"
                        ScaleY="2.2" />
          <SkewTransform AngleX="80"
                          AngleY="0" />
        </TransformGroup>
      </controls:LayoutTransformControl.Transform>

        <Grid HorizontalAlignment="Center" VerticalAlignment="Center" Margin="10">
        <TextBlock Foreground="White" Text="This is a test message." />
      </Grid>
    </controls:LayoutTransformControl>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-hawker Oh, I see. It is not the same method you used. I suppose that's maybe because the Matrix is not altered in the same order as it is with the default RenderTransform.

Copy link
Contributor Author

@Odonno Odonno Mar 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure but changing the +/- values should not be that hard.

@xyzzer may have an idea.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a quick look later and see. Seems like a bug we should fix for 3.0. It'll be a separate issue, so we don't necessarily have to hold up this PR over it.

I'll pull the update later and take a look at the new sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-hawker Should we create an issue for this and/or have you found out the bug?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample looks good, I created issue #2006 for the bug about the layout. Think we should port some of the Matrix helper functions and then use those to perform the operation.

@Odonno
Copy link
Contributor Author

Odonno commented Mar 23, 2018

@michael-hawker Yes. Now I have an image. But you will all of my designer work at my best. I think it's really ugly. I hope @nmetulev will contact his designer to make a proper one. :)

And I also a border on the example.

@skendrot
Copy link
Contributor

The image only needs to be 120x80. as is it can be compressed by about 50%.

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually like the icon :)

mc:Ignorable="d">

<Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
<controls:LayoutTransformControl Background="{StaticResource Brush-Grey-01}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an actual color here instead of a resource so it can be easily copy/pasted. Should this background go on the Border below? Do we need to set the HorizontalAlignment and VerticalAlignment on both the LayoutTraformControl and the Border?


# LayoutTransformControl

The [LayoutTransformControl](https://docs.microsoft.com/en-us/dotnet/api/microsoft.toolkit.uwp.ui.controls.layouttransformcontrol) is a control that apply Matrix transformations on any `FrameworkElement` of your application.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick grammar fix here, should be "is a control that applies Matrix transformations"


The transformations that can be applied are one of the following:

* Rotation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not use Matrix, Composite, or Translation Transform?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked. Indeed, MatrixTransform and TransformGroup are supported. But TranslationTransform is not handled (no effect) and CompositeTransform is not supported (throw exception).

Should I update the docs with warning for those not supported?

## Related Topics

- [Expander](Expander.md)
- [TransformGroup](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.media.transformgroup)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove en-us from url so the localization works.

@nmetulev
Copy link
Contributor

ping @Odonno

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just make sure to add the links to the transforms in the docs and good to merge


The transformations that can be applied are one of the following:

* Rotation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to the docs for these transforms

@nmetulev nmetulev merged commit f6a0651 into CommunityToolkit:master Apr 23, 2018
@windowstoolkitbot
Copy link

This PR is linked to unclosed issues. Please check if one of these issues should be closed: #1817

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants