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

Revert setting a default icon #15548

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Revert setting a default icon #15548

merged 2 commits into from
Feb 13, 2025

Conversation

aptkingston
Copy link
Member

Description

This can cause undesired effects for plugin authors, so I'm reverting this. We'll continue to show an error when adding icon components.

Copy link

qa-wolf bot commented Feb 13, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@aptkingston aptkingston marked this pull request as ready for review February 13, 2025 13:47
@poirazis
Copy link
Contributor

@aptkingston maybe set the default value of the property via the manifest schema of the icon component instead of the icon property so the error state can be avoided too

@aptkingston
Copy link
Member Author

@aptkingston maybe set the default value of the property via the manifest schema of the icon component instead of the icon property so the error state can be avoided too

Do you do that yourself with your icon settings - setting a default value via the manifest? Just curious if that works for plugins as well!

@poirazis
Copy link
Contributor

@aptkingston maybe set the default value of the property via the manifest schema of the icon component instead of the icon property so the error state can be avoided too

Do you do that yourself with your icon settings - setting a default value via the manifest? Just curious if that works for plugins as well!

I use the schema to set the default value and also set a default value on the component property , and yeah it was working !

Copy link
Contributor

@deanhannigan deanhannigan left a comment

Choose a reason for hiding this comment

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

LGTM!

@aptkingston aptkingston merged commit 72785cd into master Feb 13, 2025
20 checks passed
@aptkingston aptkingston deleted the revert-default-icon branch February 13, 2025 14:35
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants