Skip to content
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

Change base-select icon from background-image to svg elements #46152

Merged
merged 1 commit into from May 13, 2024

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 7, 2024

Putting the svg in a data url for background-image was not as
customizable in CSS for developers, but using an actual svg element is:
openui/open-ui#881

Bug: 40146374
Fixed: 337904202
Change-Id: If1528b3df97e31de5b86b27a6aa935223ff0f0a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5522663
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1300224}

Putting the svg in a data url for background-image was not as
customizable in CSS for developers, but using an actual svg element is:
openui/open-ui#881

Bug: 40146374
Fixed: 337904202
Change-Id: If1528b3df97e31de5b86b27a6aa935223ff0f0a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5522663
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1300224}
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@WeizhongX
Copy link
Contributor

./wpt lint --all --github-checks-text-file=/home/test/artifacts/checkrun.md
ERROR:lint:html/semantics/forms/the-select-element/stylable-select/select-appearance-dark-mode.tentative.html: testdriver.js included in a reftest test, which doesn't support testdriver.js (TESTDRIVER-IN-UNSUPPORTED-TYPE)

@WeizhongX
Copy link
Contributor

Pinged author on the CL. @KyleJu, can you help admin merge?

@KyleJu KyleJu merged commit bc5292b into master May 13, 2024
16 of 18 checks passed
@KyleJu KyleJu deleted the chromium-export-cl-5522663 branch May 13, 2024 21:05
chromium-wpt-export-bot pushed a commit that referenced this pull request May 13, 2024
testdriver is not supported in reftests:
#13183
#46152

I worked around using testdriver in some of these tests, but others
really need to use testdriver so I put a comment in them saying that the
test probably won't work until testdriver in reftests is supported.
These tests are still worth keeping not only for future support but
because they work properly when run with run_web_tests.py.

Bug: 40146374
Change-Id: I73d02e5ead3c2dd4c84300e78f6502357dd7524b
@foolip
Copy link
Member

foolip commented May 14, 2024

@WeizhongX @KyleJu since this PR added html/semantics/forms/the-select-element/stylable-select/select-appearance-dark-mode.tentative.html, the lint failure wasn't preexisting and admin merging in this case ended up blocking unrelated PRs. I'm quickfixing in #46251.

@WeizhongX
Copy link
Contributor

Ah, sorry...

Joey has made https://chromium-review.googlesource.com/c/chromium/src/+/5536157. After that we can remove those lines added to lint.ignore.

@WeizhongX
Copy link
Contributor

@foolip maybe we should close #13183 asap?

As we are planning to move to Wptrunner in Chromium CI, reftests using testdriver will not work after we switch.

@foolip
Copy link
Member

foolip commented May 14, 2024

@WeizhongX I agree doing that would be great. I don't know quite what would be involved however. Is it blocking Chromium's migration to wptrunner? cc @past

@WeizhongX
Copy link
Contributor

Not blocking, but some tests will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants