-
Notifications
You must be signed in to change notification settings - Fork 28
Add default styles for Card and CardContentControl controls #1507
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
|
|
| <StaticResource x:Key="CardStyle" ResourceKey="MaterialFilledCardStyle" /> | ||
| <StaticResource x:Key="CardContentControlStyle" ResourceKey="MaterialFilledCardContentControlStyle" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot these two lines aren't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the redundant aliases in d5866b6. The CardStyle and CardContentControlStyle aliases already exist in the "Other resources aliases" section below.
Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds missing default styles for the Card and CardContentControl controls, addressing the issue where these controls previously had no visual appearance when used without explicit styling.
Key Changes:
- Created base default styles in
Uno.Toolkit.UIfor both controls - Added Material Design v2 default styles that inherit from the Material filled variants
- Configured implicit style bindings and aliases in the Material library
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/library/Uno.Toolkit.Material/Styles/Controls/v2/_Common.xaml |
Adds implicit style bindings and aliases for Card and CardContentControl to Material theme |
src/library/Uno.Toolkit.Material/Styles/Controls/v2/CardContentControl.xaml |
Defines MaterialDefaultCardContentControlStyle based on the filled variant |
src/library/Uno.Toolkit.Material/Styles/Controls/v2/Card.xaml |
Defines MaterialDefaultCardStyle based on the filled variant |
src/Uno.Toolkit.UI/Controls/CardContentControl/CardContentControl.xaml |
Creates new base default style and template with theme resources for CardContentControl |
src/Uno.Toolkit.UI/Controls/Card/Card.xaml |
Creates new base default style and template with theme resources and content templates for Card |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Border x:Name="HoverPressed" | ||
| Background="{ThemeResource CardContentControlBorderBrushPressed}" | ||
| IsHitTestVisible="False" | ||
| Opacity="0" /> |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HoverPressed border is defined but never used in any visual states. The Pressed visual state (lines 101-107) sets opacity on HoverOverlay and FocusedOverlay but doesn't reference HoverPressed, making this element unused and potentially confusing.
| <Border x:Name="HoverPressed" | |
| Background="{ThemeResource CardContentControlBorderBrushPressed}" | |
| IsHitTestVisible="False" | |
| Opacity="0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HoverPressed border is actually used in the Pressed visual state on line 103 where it sets HoverPressed.Opacity to 1. The visual state correctly shows the pressed overlay when the control is pressed.
| AutomationProperties.AccessibilityView="Raw" | ||
| Content="{TemplateBinding IconsContent}" | ||
| ContentTemplate="{TemplateBinding IconsContentTemplate}" | ||
| Visibility="{Binding IconsContentTemplate, RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource NullToCollapsedConverter}}" /> |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The visibility is bound to IconsContentTemplate instead of IconsContent. This means the presenter will be visible even when there's no actual content, as long as the template is set. Should bind to IconsContent to match the pattern used by other content presenters (lines 125, 135, 145, 155).
| Visibility="{Binding IconsContentTemplate, RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource NullToCollapsedConverter}}" /> | |
| Visibility="{Binding IconsContent, RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource EmptyOrNullToCollapsedConverter}}" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Binding to IconsContentTemplate is intentional and matches the Material styles pattern. The Icons section is designed to only be visible when a template is explicitly provided (not when content is provided). This is consistent with how all the Material Card styles handle icons visibility (see MaterialOutlinedCardStyle, MaterialFilledCardStyle, MaterialElevatedCardStyle, etc.).
|
@copilot I need you to add the FluentTemplate to both the CardSamplePage and the CardContentControlSamplePage and add examples of the Card and CardContentControl using the new default Styles you created in the main Uno.Toolkit.UI project |
…h default styles Co-authored-by: kazo0 <4793020+kazo0@users.noreply.github.com>
Added FluentTemplate sections to both CardSamplePage.xaml and CardContentControlSamplePage.xaml in f7edb38. The examples demonstrate the default styles (DefaultCardStyle and DefaultCardContentControlStyle) created in the base Uno.Toolkit.UI project with various configurations including media, supporting text, and disabled states. |
GitHub Issue (If applicable): #
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Card and CardContentControl controls have no default styles defined. When used without explicitly specifying a style, the controls render with no visual appearance.
What is the new behavior?
Default styles are now applied automatically:
Base Uno.Toolkit.UI:
DefaultCardStyleinControls/Card/Card.xamlDefaultCardContentControlStyleinControls/CardContentControl/CardContentControl.xamlMaterial Library (v2):
MaterialDefaultCardStylebased onMaterialFilledCardStyleMaterialDefaultCardContentControlStylebased onMaterialFilledCardContentControlStyle_Common.xamlSample Pages:
FluentTemplateexamples toCardSamplePage.xamldemonstrating the default Card style with various configurations (basic, with supporting text, with media, and disabled states)FluentTemplateexamples toCardContentControlSamplePage.xamldemonstrating the default CardContentControl style with custom content templatesFollows the same pattern as Divider, NavigationBar, and Chip controls.
Note: Redundant style aliases were removed from
_Common.xamlas they already existed in the "Other resources aliases" section.PR Checklist
Please check if your PR fulfills the following requirements:
Other information
Internal Issue (If applicable):
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.