-
Notifications
You must be signed in to change notification settings - Fork 121
[IPPInAppFeedback] UI banner component design tweaks #8728
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
While we’re moving towards making everything purple (pecCkj-fa-p2), some links are kept pink, so we can modify the component for all cases.
You can test the changes from this Pull Request by:
|
| static var builtInReaderError: UIImage { | ||
| return UIImage(named: "built-in-reader-error")! | ||
| } | ||
|
|
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.
Not a blocker, empty line here
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.
Fixed 3993a92
| @@ -0,0 +1,21 @@ | |||
| { | |||
| "images" : [ | |||
| { | |||
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.
Perhaps you can apply here a single scale with the pdf file
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.
Right! Switched from SVG to PDF and universal size here: 26d3d06
|
👋 Looping you in the PR as well @vanessamaartinstt . There's a known UI issue that we're discussing at p1674112248357019-slack-C0354HSNUJH : Edit: This is resolved. |
Generated by 🚫 dangerJS |
|
Resolved the issues, as well as the text moving below the dismiss button: We were using an Ready for re-review @toupper 🙇 |
|
|
||
| func createLabelHolderStackView() -> UIStackView { | ||
| labelHolderStackView.addArrangedSubviews([ | ||
| createSeparatorView(height: 48, width: 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.
Could we add these values into constants? That way we can reuse them, and we know what they stand for
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.
Definitely! 4579e44
|
Great job @iamgabrielma ! Approved 🚢 |

Part of #8530
Closes: #8674
Description
This PR updates the In-Person Payments Feedback banner UI design to make it closer to the design proposal.
Changes
commentContenticon.purplecolor for the Action links with.pink(confirmed with design at C0354HSNUJH/p1674112248357019-slack-C0354HSNUJH)createBorderView()in the mainUIStackView@toupper as you mentioned in the prior PR, the banner text doesn't go below the
Xicon for dismissal, but I haven't found a way to make this happen with the current implementation without having to modify it entirely. Happy to hear if you have any pointer or recommendation! 🙇Testing instructions
On a store with COD enabled, and set to US or CA: