Skip to content

Conversation

@Mathieu-COSYNS
Copy link
Contributor

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
    • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

The clear button color remains gray

Issue URL:

#19178

What is the new behavior?

The clear button color match the color set by --ion-color-step-600

Does this introduce a breaking change?

  • Yes
  • No

Other information

In the bug report SidiBecker mention that it is not possible to change the color of the clear button icon. This is due to the fact that css variables can not be used inside an url("...") function in css. My fix use the mask-image property and allow the color to be set via background-color. For browsers who are not supporting mask-image the old behavior is used as a fallback.

I am new to this project. I had trouble to understand and run tests. I don't know if it's the way to go but I simply had a new element in core/src/components/input/test/basic/index.html to showcase my changes.

@liamdebeasi
Copy link
Contributor

Thanks for the PR! We appreciate the work you put into creating this fix. We would like to resolve this issue by using an IonIcon instead of continuing to use an SVG background. The benefit of using the Ionicon is that it fixes the reported issue as well as an issue regarding CSP violations. (See: ionic-team/ionicons#1025. This is fixed in Ionicons but would not be fixed by continuing to use this SVG background approach).

I am going to close this in favor of a newer PR here: #26354. I will give you co-author credit if this PR is merged. Let me know if you have any questions.

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

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants