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
feat: support more color formats for backgroundColor
#31868
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.
API LGTM
API LGTM |
e5097a6
to
7c84127
Compare
c2dcb17
to
f797a7a
Compare
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 have a couple of nit suggestions but overall LGTM
f797a7a
to
6790e31
Compare
Discussed with the API WG and we'll keep the ARGB format to avoid breaking existing apps, and drop the format option from getBackgroundColor. |
6790e31
to
7186b9d
Compare
42a82c0
to
4987a7a
Compare
Co-authored-by: Jeremy Rose <jeremya@chromium.org>
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.
lgtm with nits
Release Notes Persisted
|
I have automatically backported this PR to "18-x-y", please check out #33364 |
Description of Change
Refs #31019.
In #30193, we unintentionally refactored
WebPreferences
such thatbackgroundColor
no longer accepted CSS color values likered
. However, that only worked for some types ofbackgroundColor
methods, since others did enforce that the passed value be hex.This PR thus adds more official support for the following formats:
And as a bonus removes some duplicate code from our
color_util
file.Checklist
npm test
passesRelease Notes
Notes: Added support for more color formats in
setBackgroundColor
.