smithfield-studio / acf-svg-icon-picker

Creates an ACF SVG Icon Picker field
MIT License
12 stars 1 forks source link

Proposals for fork #1

Open Levdbas opened 4 months ago

Levdbas commented 4 months ago

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.


@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

mike-sheppard commented 4 months ago

hey @Levdbas sounds great!

No problems on my side with any of these changes. I'll jump on #2 today + feel free to add a PR for the others, and I'll get 'em merged in :)

mike-sheppard commented 4 months ago

@Levdbas mind testing the dev-main branch version your side - when reverting the ACF name back to icon-picker I can't seem to get my fields to work. Just want to check if it's working for you in ACF 6.3 or something my side?

Levdbas commented 4 months ago

I'll do that first thing tomorrow @mike-sheppard

mike-sheppard commented 4 months ago

Thanks, hopefully all working ok now 🤞(pushed up some quick fixes). If all's ok your side, I'll publish the v3.0.0 release

Levdbas commented 4 months ago

Only seeing the main branch, no dev-main. Let me know!

mike-sheppard commented 4 months ago

ah yeah sorry, yep main (dev-main if using it via composer)

"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",
}
Levdbas commented 4 months ago

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

Dennisscholten commented 3 months ago

Great work! Sadly, it doesn't solve the following: when having a flexible field layout with a svg icon picker field in it, I am unable to remove the field. This is the JS error I'm getting (screenshot):

Scherm­afbeelding 2024-06-05 om 09 54 59
mike-sheppard commented 3 months ago

hey 👋 @Dennisscholten, @Levdbas pushed another fix to update the ACF field name so it should hopefully be working everywhere now (since v3.1.0)

Edit: Aha, I see you've discussed in #15, perfect.