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

IconElement implementation #543

Merged

Conversation

IvanDmitriev1
Copy link
Collaborator

Implements a IconElement control almost like in winui 3.
Replaces SymbolIcon with IconElement.

Pull request type

  • Update
  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes

Other information

@github-actions github-actions bot added the controls Changes to the appearance or logic of custom controls. label Feb 23, 2023
@github-actions github-actions bot added the styles Topic is related to styles label Feb 24, 2023
@IvanDmitriev1 IvanDmitriev1 marked this pull request as ready for review February 24, 2023 16:00
@IvanDmitriev1
Copy link
Collaborator Author

@pomianowski I hope you haven't forgotten about this interesting pull-request.

@pomianowski pomianowski self-assigned this Mar 15, 2023
@pomianowski pomianowski added this to the Base controls complete milestone Mar 15, 2023
Copy link
Member

@pomianowski pomianowski left a comment

Choose a reason for hiding this comment

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

Hey @IvanDmitriev1 thank you again for your huge contribution and effort. You did a great job modifying the icons by the way changing strong dependencies on specific classes. Apart from a few minor things, the code looks fine to me.

However, looking at your recent changes, I think it's probably better to put all the control-related classes in one Controls directory. It may be a lot stuff in one place, but I think it will be easier for the end user to use, similar to what is done in WPF.

As for presenting icons, I think ContentPresenter is a better than ContentControl for that.

<ContentPresenter
    x:Name="ControlIconLeft"
    Grid.Column="0"
    Margin="{StaticResource TextBoxLeftIconMargin}"
    VerticalAlignment="Top"
    Content="{TemplateBinding Icon}"
    Focusable="False"
    TextElement.FontSize="16"
    TextElement.Foreground="{TemplateBinding Foreground}"
    Visibility="Visible" />

Eventually, ContentControl uses ContentPresenter to present content, so we can consider ContentPresenter as more ligthweight
https://github.com/dotnet/wpf/blob/main/src/Microsoft.DotNet.Wpf/src/Themes/XAML/ContentControl.xaml#LL17C21-L17C40

src/Wpf.Ui.Gallery/Views/Pages/BasicInput/AnchorPage.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui.Gallery/Views/Pages/DashboardPage.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui/Styles/Controls/Snackbar.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui/Styles/Controls/TextBox.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui/Styles/Controls/TextBox.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui/Styles/Controls/TitleBar.xaml Outdated Show resolved Hide resolved
src/Wpf.Ui/Styles/Controls/TreeViewItem.xaml Outdated Show resolved Hide resolved
@IvanDmitriev1
Copy link
Collaborator Author

IvanDmitriev1 commented Mar 16, 2023

As for presenting icons, I think ContentPresenter is a better than ContentControl for that.

<ContentPresenter
    x:Name="ControlIconLeft"
    Grid.Column="0"
    Margin="{StaticResource TextBoxLeftIconMargin}"
    VerticalAlignment="Top"
    Content="{TemplateBinding Icon}"
    Focusable="False"
    TextElement.FontSize="16"
    TextElement.Foreground="{TemplateBinding Foreground}"
    Visibility="Visible" />

Eventually, ContentControl uses ContentPresenter to present content, so we can consider ContentPresenter as more ligthweight https://github.com/dotnet/wpf/blob/main/src/Microsoft.DotNet.Wpf/src/Themes/XAML/ContentControl.xaml#LL17C21-L17C40

ContentControl is used for proper foreground inheritance. And also with ContentPresenter it didn't work, but right now it seems to work. Well, I need to check everything again.

@pomianowski pomianowski merged commit 91a7089 into lepoco:development Mar 17, 2023
@IvanDmitriev1 IvanDmitriev1 deleted the IconElement-implementation branch March 17, 2023 08:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
controls Changes to the appearance or logic of custom controls. styles Topic is related to styles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants