-
Notifications
You must be signed in to change notification settings - Fork 120
[Booking] Update booking badges colours #16307
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
Conversation
03a0ae7 to
a56c570
Compare
|
|
itsmeichigo
left a comment
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.
This works as expected. I left some suggestions in the comments.
| .overlay( | ||
| Circle() | ||
| .stroke(Color.white, lineWidth: Layout.borderLineWidth) | ||
| .opacity(customizations.bordered ? 1 : 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.
Nit: why don't we apply this only if bordered is true?
| .overlay( | |
| Circle() | |
| .stroke(Color.white, lineWidth: Layout.borderLineWidth) | |
| .opacity(customizations.bordered ? 1 : 0) | |
| ) | |
| .if(customizations.bordered) { view in | |
| view.overlay( | |
| Circle() | |
| .stroke(Color.white, lineWidth: Layout.borderLineWidth) | |
| ) | |
| } |
Similar suggestion for the case below.
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.
Done here
| "BookingAttendanceStatus.checkedIn", | ||
| value: "Checked In", | ||
| value: "Checked-in", |
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.
Instead of manually updating the strings in Localizable.strings, you can update the key for the updated string. The new key and value will be generated automatically in Localizable.strings.
| "BookingAttendanceStatus.checkedIn", | |
| value: "Checked In", | |
| value: "Checked-in", | |
| "bookingAttendanceStatus.checkedIn.title", | |
| value: "Checked-in", |
Similar suggestion for the other change below.
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.
I see, but changing the key would mean that the intention of the text is different, but this is not the case now. Just the text is changing. In that sense, I'd keep the key and only change the value.
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.
Yeah I get your point, you might not need new translation for this text. I'm good with keeping this as-is.

Closes: WOOMOB-1618
Description
Adds coloring to the booking status. Currently, only the
No-showandUnpaidare colored, but we can easily extend this if needed.Test Steps
No-showandUnpaidare color "yellowish".Screenshots
RELEASE-NOTES.txtif necessary.