Skip to content

Conversation

@s3rgiosan
Copy link
Member

@s3rgiosan s3rgiosan commented Jun 28, 2025

Description of the Change

This PR adds a new "Icon color" setting, allowing editors to control the SVG fill color. It also removes text color support, as it's redundant in this icon-specific context.

Before

Screenshot 2025-06-28 at 17 31 02

After

Screenshot 2025-06-28 at 17 31 30

How to test the Change

Changelog Entry

Added - fillColor support with Icon color settings

Credits

Props @s3rgiosan

Checklist:

@s3rgiosan s3rgiosan requested a review from darylldoyle June 28, 2025 16:37
@dkotter dkotter added this to the 2.4.0 milestone Jun 30, 2025
@dkotter dkotter merged commit 50510fc into 10up:develop Jun 30, 2025
13 checks passed
@s3rgiosan s3rgiosan deleted the feature/add-fill-color-control branch July 2, 2025 12:49
@dkotter dkotter modified the milestones: 2.4.0, 2.3.2 Jul 14, 2025
@dkotter
Copy link
Collaborator

dkotter commented Jul 15, 2025

@s3rgiosan Apologies for the ping here after this is already merged but in doing final testing today in advance of doing a release (#263) I noticed that if I set a custom icon color, that shows properly in the editor but doesn't show on the front-end (which I must have missed when testing this previously).

It seems on the front-end (in register.php) we still use the textColor attribute, which no longer exists and needs updated to I believe iconColor. In addition, wondering if we need to consider supporting sites that did previously have textColor set, instead of just replacing that with iconColor. Basically wondering if we need a fallback to look at textColor first and use it if it exists and if not, fallback to iconColor?

Wondering if you have time to open a new PR to address this to help unblock the release?

@dkotter dkotter mentioned this pull request Jul 15, 2025
16 tasks
@s3rgiosan
Copy link
Member Author

@dkotter I'll work on a fix as soon as possible

@s3rgiosan
Copy link
Member Author

s3rgiosan commented Jul 18, 2025

@dkotter This was addressed here.

I reverted the change that added the icon color. While using the icon color makes more sense than relying on the text color for the SVG, the goal here was to unblock this release. We can revisit and properly add icon color support later, including fallback to text color.

Let me know if you'd prefer to hold off and address the icon color implementation before releasing the new version.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants