feat: 🎸 [JIRA:HCPSDKFIORIUIKIT-2707] SwiftUI RatingControl Enhancement #794
Conversation
SwiftUI RatingControl enhancement with value label and review count label ✅ Closes: HCPSDKFIORIUIKIT-2707
|
|
||
| This `RatingControl` is read-only. Each rating star is in large scale. | ||
| */ | ||
| case standardLarge |
There was a problem hiding this comment.
I'd like to use the Bool variable isLarge with the UIKit FUIRatingControl. By default, it is set to false for the read-only state and always true for the editable and disabled states. This approach is more simple to me when a smaller size is needed for the editable or disabled states, or for any new states that may be introduced in the future. For us, this means that we do not need to add too many styles, and for the App team, it makes it easy to understand that there are two sizes for each state.
I just want to let you know about the difference between your API and mine. However, I am definitely okay with your implementation for these two new styles here for SWiftUI Rating Control.
There was a problem hiding this comment.
@JunSong-SH I was originally using the isLarge property. However, later I removed it to reduce the number of properties. I think there are too many properties already.
| configuration.getOnImageView() | ||
| } else { | ||
| configuration.getOffImageView() | ||
| if configuration.showsValueLabel, self.getReadOnly(configuration) { |
There was a problem hiding this comment.
I am a little confused about the left value for the editable/disabled state. The design spec mentions 'Left value (optional) except for editable and disabled variants.'. From my understanding, this means that the left value is required/mandatory for the editable and disabled states. I have consulted the designer about this in Figma.
There's a high probability that my understanding of the above sentence might be incorrect. But, just want to let you know this. :-)
There was a problem hiding this comment.
@JunSong-SH I have not thought about the behavior you mentioned above. Let's wait for the designer to clarify the behavior.
| } | ||
| } | ||
| if configuration.showsValueLabel, !self.getReadOnly(configuration) { |
There was a problem hiding this comment.
Now, I understand the meaning of valueLabel and reviewCountLabel you always mentioned during our discussion But, for me, I would think it is the value concept for displaying on left and right. And, I feel it is not very straightforward. For example, here, if App team want to customize the value displayed on the right, they must to know when they should set the value for one of 'ratingValueFormat', 'averageRatingFormat', 'reviewCountFormat' and 'reviewCountCeilingFormat'.
For me, I'd like to use more simple or straightforward logic on UIKit: To use two 'showXXXLabel' bool flag to control label display and use two 'XXXLabelFormat' for customization whatever we call it 'leftLabel/leadingLabel/valueLabel' or 'rightLabel/trailingLabel/reviewCountLabel'(Now, I use valueLabel and reviewCountLabel base on our discussion before). Absolutely, 'reviewCount' and 'reviewCountCeiling' were needed. Then, App team just to know there are two 'XXXLabelFormat' for customization and the default value for those two labels was that the label on left will use default formatted rating value or average rating value according to the style(confirming with designer about should display value for NON-readonly state). For the value of label on right, by default, to use '3 of 5' or '2.3 of 5' if there is no review count and use '9 review' or '8+ reviews' if there has reviewCount or reviewCount>reviewCountCeiling.
Certainly, I just want to let you know about the different between your API and mine. :-)
There was a problem hiding this comment.
@JunSong-SH Thanks for letting me know the differences of the APIs. I understand your reasonings. However, I think it is better to have a meaning to the label instead of the left and right. What if the designer changing the position to both on the right in the future? Also, in the large font categories, the labels are on the top and bottom.
I think it is fine to have different APIs.
| let diff = averageRating - CGFloat(i) | ||
| if diff < 0.3 { | ||
| items.append(RatingItem(isOn: false, isHalf: false)) | ||
| } else if diff < 0.7 { |
There was a problem hiding this comment.
See FIORITECHE1-8222, web component use 1.0-1.2 -> 1, 1.3-1.7 -> 1.5 and 1.8-1.9 -> 2. So, here, shouldn't diff <= 0.7 or diff < 0.8?
There was a problem hiding this comment.
Yes, you're right. I'll change it. Actually, I think use half star for 0.25 to 0.75 is more reasonable, since it is a Float. I'll check with the designer. Let's merge this code first and will change later with the changes needed for large text font.
Sources/FioriSwiftUICore/_ComponentProtocols/CompositeComponentProtocols.swift
Outdated
Show resolved
Hide resolved
Sources/FioriSwiftUICore/_ComponentProtocols/CompositeComponentProtocols.swift
Outdated
Show resolved
Hide resolved
✅ Closes: JIRA:HCPSDKFIORIUIKIT-2707
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _HalfStarImageComponent { | ||
| //// The image to be used for "half" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OffStarImageComponent { | ||
| //// The image to be used for "Off" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
| // sourcery: BaseComponent | ||
| // sourcery: importFrameworks = ["FioriThemeManager"] | ||
| protocol _OnStarImageComponent { | ||
| /// The image to be used for "On" rating star. |
There was a problem hiding this comment.
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
SwiftUI RatingControl enhancement with value label and review count label
✅ Closes: HCPSDKFIORIUIKIT-2707