Form Customizer: Refactor to WP and PHPStan standards, add tests. #84
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kraftbj
requested changes
Mar 25, 2025
Contributor
kraftbj
left a comment
kraftbj
left a comment
There was a problem hiding this comment.
All in all looks good. Made a couple of minor suggestions.
includes/forms/form-customizer.php
Outdated
| * It manages preview values, fields, and errors for the customizer interface, and handles | ||
| * saving ACF data when customizer changes are applied. | ||
| * | ||
| * @package Advanced Custom Fields |
Contributor
There was a problem hiding this comment.
Suggested change
| * @package Advanced Custom Fields | |
| * @package wordpress/secure-custom-fields |
includes/forms/form-customizer.php
Outdated
| * @param array $new_instance (array) widget settings. | ||
| * @param array $old_instance (array) widget settings. | ||
| * @param object $widget (object) widget info. | ||
| * @return array $instance Widget settings. |
Contributor
There was a problem hiding this comment.
This spacing seems off to me.
Suggested change
| * @return array $instance Widget settings. | |
| * | |
| * @return array $instance Widget settings. |
includes/forms/form-customizer.php
Outdated
| * | ||
| * @param $customizer (object) | ||
| * @return $value (mixed) | ||
| * @param Object $customizer WordPress object. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @param Object $customizer WordPress object. | |
| * @param WP_Customize_Manager $customizer Customizer object. |
includes/forms/form-customizer.php
Outdated
| * | ||
| * @param $customizer (object) | ||
| * @return n/a | ||
| * @param Object $customizer WordPress object. |
Contributor
There was a problem hiding this comment.
Suggested change
| * @param Object $customizer WordPress object. | |
| * @param WP_Customize_Manager $customizer Customizer object. |
kraftbj
added
[Status] Awaiting Author Response
and removed
[Status] Needs Review
labels
cbravobernal
force-pushed
the
refactor/form-customizer
branch
from
efcc8ae to
843f80e
Compare
kraftbj
approved these changes
Mar 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR works on
form-customizer.phpfile. This one seems to be somewhat legacy, but tests are passing both with the old and the refactor code.The class name change should not have side effects, as is only called in this file. Still may be "risky".
Anyway, feel free to leave feedback. I hope I didn't miss anything.