Update Scan card and fill button with credit finder symbol [chromium/src : main]
Qihui Zhao (Gerrit)
Qihui Zhao added 1 comment![]( "Open in Gerrit")
Hi Tommy, please help to review, thanks!
Related details
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qihui Zhao (Gerrit)
Qihui Zhao added 1 comment![]( "Open in Gerrit")
if (image) {I updated around here to make the image/symbol in white color but failed, could you please help, thanks!
Related details
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Martino (Gerrit)
Tommy Martino added 2 comments![]( "Open in Gerrit")
This CLLooks like maybe your CL description didn't save?
if (image) {I updated around here to make the image/symbol in white color but failed, could you please help, thanks!
I'm not familiar with this code, and I'm also not sure how the owners will want these APIs to be updated to support primary button images (e.g., will they want to set the color internally like you're doing here, or prefer that the users of this class set the color of `image` before passing it to the config?).
This would be a good time to reach out to one of the owners of this file and discuss what the right way to achieve this would be.
Related details
- Qihui Zhao
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qihui Zhao (Gerrit)
Qihui Zhao added 2 comments![]( "Open in Gerrit")
Hi Gauthier, please help to review ios/chrome/browser/shared/ui/symbols/ and ios/chrome/common/ui/button_stack/, let me know if it's the correct way to add primary button image, thanks!
Looks like maybe your CL description didn't save?
- Gauthier Ambard
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard added 1 comment![]( "Open in Gerrit")
if (image) {Tommy MartinoI updated around here to make the image/symbol in white color but failed, could you please help, thanks!
I'm not familiar with this code, and I'm also not sure how the owners will want these APIs to be updated to support primary button images (e.g., will they want to set the color internally like you're doing here, or prefer that the users of this class set the color of `image` before passing it to the config?).
This would be a good time to reach out to one of the owners of this file and discuss what the right way to achieve this would be.
I am not sure to understand what you want to achieve as there is no bug number.
Could you add a screenshot of what you get and what you want to achieve?
Related details
- Qihui Zhao
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Qihui Zhao (Gerrit)
Qihui Zhao added 1 comment![]( "Open in Gerrit")
if (image) {Tommy MartinoI updated around here to make the image/symbol in white color but failed, could you please help, thanks!
Gauthier AmbardI'm not familiar with this code, and I'm also not sure how the owners will want these APIs to be updated to support primary button images (e.g., will they want to set the color internally like you're doing here, or prefer that the users of this class set the color of `image` before passing it to the config?).
This would be a good time to reach out to one of the owners of this file and discuss what the right way to achieve this would be.
I am not sure to understand what you want to achieve as there is no bug number.
Could you add a screenshot of what you get and what you want to achieve?
oops, I forgot to attach screenshot and mock, updated in the CL description, thanks!
Related details
- Gauthier Ambard
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard added 1 comment![]( "Open in Gerrit")
config.image = self.configuration.primaryActionImage;@ewa...@chromium.org why did we restrict the buttons to have only checkmark or spinner? Are we supposed to be able to set images?
Related details
- Ewann Pellé
- Qihui Zhao
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ewann Pellé (Gerrit)
Ewann Pellé added 1 comment![]( "Open in Gerrit")
config.image = self.configuration.primaryActionImage;@ewa...@chromium.org why did we restrict the buttons to have only checkmark or spinner? Are we supposed to be able to set images?
To enforce strict visual uniformity on primary ChromeButton. https://crsrc.org/c/ios/chrome/common/ui/util/chrome_button.mm?q=setPrimaryButtonImage
the image could be set in the subclass after viewDidLoad like
```
UIButtonConfiguration* config = self.primaryActionButton.configuration;
config.image = DefaultSymbolTemplateWithPointSize(
kCreditCardFinderActionSymbol, kSymbolActionPointSize);
self.primaryActionButton.configuration = config;
```
Related details
- Qihui Zhao
- Tommy Martino
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |