-
Notifications
You must be signed in to change notification settings - Fork 4
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
Proposals for fork #1
Comments
hey @Levdbas sounds great! No problems on my side with any of these changes. I'll jump on |
@Levdbas mind testing the |
I'll do that first thing tomorrow @mike-sheppard |
Thanks, hopefully all working ok now 🤞(pushed up some quick fixes). If all's ok your side, I'll publish the |
Only seeing the main branch, no dev-main. Let me know! |
ah yeah sorry, yep "repositories": {
"acf-svg-icon-picker": {
"type": "git",
"url": "https://github.com/smithfield-studio/acf-svg-icon-picker"
}
}
...
"require": {
"smithfield-studio/acf-svg-icon-picker": "dev-main",
} |
Hi @mike-sheppard , your changes work on one of my existing projects. Only thing I had to do was reactivate the plugin after installation. I made some other iterations in #11 for you to review |
hey 👋 @Dennisscholten, @Levdbas pushed another fix to update the ACF field name so it should hopefully be working everywhere now (since Edit: Aha, I see you've discussed in #15, perfect. |
Hi @mike-sheppard ,
I went over the plugin and these are the most important changes I would like to suggest, ordered from the most important to least. As said before, I can provide PR's for this but I first would like to discuss with you how you see my suggested changes.
Revert back field name, to make it easier for people to transition to this fork. The field name does not collide with ACF. #4(reverted the revert 😅)initialize
method instead of__construct
in the ACF field registration. #5@Levdbas Edit: Especially the first two would be a priority for me personally, I have 15 projects that are currently running this plugin where the icon picker is currently not working and I am not really fond of downgrading ACF if there is a solution at hand. I'd rather swap out the plugin via Composer. 👯
@mike-sheppard Edit: converted the list into individual issues
The text was updated successfully, but these errors were encountered: