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

[Experiment] Shimmer #379

Closed
wants to merge 32 commits into from
Closed

[Experiment] Shimmer #379

wants to merge 32 commits into from

Conversation

niels9001
Copy link
Collaborator

@niels9001 niels9001 commented Mar 1, 2023

Adding the Shimmer control from the Store. See: #381

Shimmerrr

@michael-hawker
Copy link
Member

@Arlodotexe found out what happens if you try and run WASM head, it just says that the underlying project isn't compatible with the TFM... But since we don't build those the same way in the CI, it works fine here. So just something we can improve later for the OpenSolution solution...

@michael-hawker michael-hawker added the experiment 🧪 Used to track issues that are experiments (or their linked discussions) label Mar 7, 2023
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.

Looks good, tested on UWP and WindowsAppSDK without issue (so cool to see UWP code just working over there via Labs).

Have some suggestions for the property names and future investigations we should track.

FYI @JustinXinLiu

Comment on lines +32 to +40
<Grid Width="80"
Height="20"
HorizontalAlignment="Center">
<TextBlock x:Name="Title"
FontWeight="SemiBold"
Text="This is a title"
Visibility="{x:Bind HasLoaded, Mode=OneWay}" />
<labs:Shimmer Visibility="{x:Bind local:ShimmerSample.ReverseVisibility(Title.Visibility), Mode=OneWay}" />
</Grid>
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty similar to how the Store uses it, eh? Though they have a faux-item template and swap it out later or something with a selector? (We should probably show another example of how that works for an items collection.)

We should create a tracking issue for listing out next steps. (close to defining how we want to track independent issues tied to an experiment)

It'd be really cool to investigate if we could make this easier to use, though I think that'll have to be an 8.1 thing, and this is basically just the raw component that knows how to fill a space and shimmer, eh?

Comment on lines +7 to +8


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

extra newline

#endif


namespace CommunityToolkit.Labs.WinUI;
Copy link
Member

Choose a reason for hiding this comment

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

Should we try the new CommunityToolkit.WinUI.Controls namespace here yet?

/// A generic shimmer control that can be used to construct a beautiful loading effect.
/// </summary>
[TemplatePart(Name = PART_Shape, Type = typeof(Rectangle))]
#pragma warning disable CA1001 // Types that own disposable fields should be disposable
Copy link
Member

Choose a reason for hiding this comment

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

Should be resolved once we fix #377 eh?

private bool _initialized;
private bool _animationStarted;

#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.
Copy link
Member

Choose a reason for hiding this comment

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

We should probably make everything probably nullable?

Comment on lines +30 to +34
<ControlTemplate TargetType="labs:Shimmer">
<Border x:Name="Shape"
Background="{TemplateBinding Background}"
CornerRadius="{TemplateBinding CornerRadius}" />
</ControlTemplate>
Copy link
Member

Choose a reason for hiding this comment

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

So little XAML. As another follow-up for the tracking issue, maybe should investigate if we can go the ContentPresenter route like we did with Constrained Box and do some funky stuff like that... Trying to think what the most lightweight element we can inherit from would be since Border is sealed... As we could take a couple of approaches, but would be nice to remove the XAML template as then would be super light-weight.

ContentPresenter would lend itself to just wrapping something else, though I know our attached shadow like effect route could be interesting as well. Probably different use cases depending on what and how things are being loaded. We'll definitely have to play around more with these ideas in the future.

@niels9001 niels9001 force-pushed the user/niels9001/shimmer branch from 1e0ac28 to 1d29f9c Compare March 8, 2023 09:25
@niels9001
Copy link
Collaborator Author

Closing this for: #391

@niels9001 niels9001 closed this Mar 8, 2023
@niels9001 niels9001 deleted the user/niels9001/shimmer branch June 22, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experiment 🧪 Used to track issues that are experiments (or their linked discussions)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants