Some ideas #2
Some ideas #2
Conversation
According to the doc., the behaviour of this plugin could be achieve with the Select field through the filters `acf/load_value` in PHP and `select2_args` in JS (and some other little things maybe). It seems logical that the code follows the Select field one.
This new behaviour offer 2 new ways to hook into `acf_svg_icon_filepath`,
and follows the ACF filters methodology.
1. `acf_svg_icon_filepath` - filter for every field.
2. `acf_svg_icon_filepath/name={$field_name}` - filter for a specific field based on it's name.
3. `acf_svg_icon_filepath/key={$field_key}` - filter for a specific field based on it's key.
This feature allows us to translate all SVG labels through
the filter `acf_svg_icon_data` in 3 ways:
1. `acf_svg_icon_data` - filter for every field
2. `acf_svg_icon_data/name={$field_name}` - filter for a specific field based on it's name
3. `acf_svg_icon_data/key={$field_key}` - filter for a specific field based on it's key
By the way, you can also use this filter to reduce the list of SVG symbols ;)
- Use `acf.select2` instead of `select2` to be sure to have the same behaviour and styles than ACF Select fields with stylised UI. It's also handier than the previous method ;) - Remove the SVG #ID from HTML classes because it can have the same name than a WP CSS class selector (e.g.: `.arrow`) and break the layout. - Change some declarations and a selector in CSS.
|
Hello @7studio, we will have a look soon. |
|
IS it possible to make a pull request only with the modifications ? here you make a pull request with big files changes but there is no modifications on these files really, maybe mod changed ? |
|
Hi @Rahe, I'm a bit disappointed because the code is totally different from the beginning 😒 You are right, the default diff of GitHub is difficult to read/understand but my pull request contains 12 commits and it would be more interesting to check the changes commit by commit. The main big files changes are due to the commits 6610a86 and 22feea6. I don't know what I can do to help your review sorry 😔 |
|
Hi, we will review your commits for the project. Maybe we will not take as is and integrate only thé changes. |
|
It's on process : https://github.com/BeAPI/acf-svg-icon/tree/pr-2 |
|
Hi @MaximeCulea Thank you for taking into account my work 😊 From the beginning, I'm confused and I don't understand why you don't merge all these changes via my PR instead of rewriting your own commits. I'm really interested in knowing your opinion about my process/work in this PR and this lack of understanding for my new/next contributions 😉 |
|
Hello, We are reviewing all the code and checking if everything is okay. We prefer to focus on adding the feature than change everything. Here maybe (we do not have reviewed everything) some code parts have to be rewrited. But be assured we value your contribution and work on this :) |
|
Thank you for your answer Nicolas. I understand your point of view and I agree with you 😊 However, I don't totally agree with your idea about the |
Since its version 5.6.0, ACF has switched to Select2 v4 and thus has introduced some break changes even if we used its `acf.select2` object. This is due to the fact that ACF and its object don't abstract Select2 options by its own and haven't chosen to handle the renaming of the formatting options `formatResult` and `formatSelection` (cf.: https://t.co/RhEAwy8Fed?amp=1). As ACF allows to choose Select2 version (3 or 4) and handles this detail in its JS process, we should handle this point in JS and CSS as well.
Hi @MaximeCulea,
I hope the WCEU was interesting 😉
I tried to slice my PR into little commits to allow you to pick up some of them if you don't want particular changes. Do not hesitate to give me your feedbacks. It's always interesting to improve my skills and my understanding of PHP or WP (I'm not a back-end dev) 😊
We could go further in the process of the plugin: ajax loading, disabled options, etc. But IMO that is not bad as it is.
The only thing that I wonder about is the use of
wp_get_cacheinstead ofget_transientbut maybe it doesn't matter.Fixes #1.
Have a good day,
Xavier.