-
Notifications
You must be signed in to change notification settings - Fork 416
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
Rating view #1706
Rating view #1706
Conversation
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.
Thanks for moving this forward, some code style points
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Behaviors/ProgressBarAnimationBehavior.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
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.
Thanks @Eel2000! Could you fix these Find + Replace typos?
src/CommunityToolkit.Maui/Behaviors/ProgressBarAnimationBehavior.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.UnitTests/Converters/ByteArrayToImageSourceConverterTests.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Behaviors/Validators/ValidationBehavior.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Extensions/ColorAnimationExtensions.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Extensions/ValueConverterExtension.shared.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Pedro Jesus <[email protected]>
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.
Thanks @Eel2000!
We should be using read-only properties instead of expression-bodied properties.
I also noted that we still need to add descriptive XML comments, but we should wait to complete these until the Proposal has been approved.
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui.Core/Primitives/Defaults/RatingShape.cs
Outdated
Show resolved
Hide resolved
|
||
|
||
/// <summary>Star Color Bindable property.</summary> | ||
public static readonly BindableProperty FilledBackgroundColorProperty = BindableProperty.Create(nameof(FilledBackgroundColor), typeof(Color), typeof(RatingView), defaultValue: Colors.Yellow, propertyChanged: OnBindablePropertyChanged); |
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.
What is preferable Backgroud as Brush or BackgroundColor as Color?
Background allows more customization like gradient background
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Maui/Views/RatingView/RatingView.Shared.cs
Outdated
Show resolved
Hide resolved
@dotnet-policy-service agree
@dotnet-policy-service agree |
Closing this PR in favor of the new RatingView PR: #2191 |
Description of Change
Adding the Rating view to the community toolkit, this is the initial code which requires some validations.
this code was ported from an existing repo with changes made based on suggestions from the team. It needs some validations(suggestions) before going any further. Unit tests, samples, and the code doc consolidation are not yet. Will be added after reviews and any new suggestion.
Linked Issues
PR Checklist
approved
(bug) orChampioned
(feature/proposal)Unit tests omitted because i wanted to validate first the existing code which was ported from an existing repo before writing
unit tests.
Need some code validation before adding the sample page in the samples app.
main
at time of PRAdditional information