Refactor custom status module views #736
Merged
Conversation
This file contains 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
hanifn
reviewed
Jun 28, 2024
| $description = ( isset( $_POST['description'] ) ) ? wp_strip_all_tags( $_POST['description'] ) : $custom_status->description; | ||
| } | ||
|
|
||
| include __DIR__ . '/views/edit-status.php'; |
There was a problem hiding this comment.
Unless it's a shared/common view file, I usually default to using include_once to ensure no one accidentally include the file elsewhere. Would that fit the scenario here?
There was a problem hiding this comment.
I like that, better to not keep including it again and again.
hanifn
reviewed
Jun 28, 2024
| } else { | ||
| $custom_status_list_table = new EF_Custom_Status_List_Table(); | ||
| $custom_status_list_table->prepare_items(); | ||
| include __DIR__ . '/views/configure.php'; |
There was a problem hiding this comment.
I like that, better to not keep including it again and again.
hanifn
reviewed
Jun 28, 2024
| <form class="basic-settings" action="<?php echo esc_url( $this->get_link( array( 'action' => 'change-options' ) ) ); ?>" method="post"> | ||
| <?php settings_fields( $this->module->options_group_name ); ?> | ||
| <?php do_settings_sections( $this->module->options_group_name ); ?> | ||
| <?php echo '<input id="edit_flow_module_name" name="edit_flow_module_name" type="hidden" value="' . esc_attr( $this->module->name ) . '" />'; ?> |
There was a problem hiding this comment.
I think we can avoid echoing out the entire input field if we instead just echo out the module name.
Like this:
Suggested change
| <?php echo '<input id="edit_flow_module_name" name="edit_flow_module_name" type="hidden" value="' . esc_attr( $this->module->name ) . '" />'; ?> | |
| <input id="edit_flow_module_name" name="edit_flow_module_name" type="hidden" value="<?php echo esc_attr( $this->module->name ); ?>" /> |
hanifn
reviewed
Jun 28, 2024
| </div> | ||
|
|
||
| <?php wp_nonce_field( 'custom-status-add-nonce' ); ?> | ||
| <?php echo '<input id="action" name="action" type="hidden" value="add-new" />'; ?> |
There was a problem hiding this comment.
Not really necessary to echo this line out I think
Suggested change
| <?php echo '<input id="action" name="action" type="hidden" value="add-new" />'; ?> | |
| <input id="action" name="action" type="hidden" value="add-new" /> |
hanifn
approved these changes
Jun 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR is currently based on @ingeniumed's work in #734. Please review after that PR has been merged.
Some cleanup for the custom status module, as initial work to extend it. There were a few main changes:
The
print_configure_view()function had a lot of responsibilities, embedded HTML, and indentation. I moved the two HTML sections (configure page, edit status page) into separate view files and simplified the function.Fixed some PHPCS warnings from using
$_GETand$_POSTvariables inprint_configure_view()with some rewrites and somephpcs:ignores.Updated i18n functions to their escaped versions (
_e()→esc_html_e()) where appropriate.Moved one string (
ef_confirm_delete_status_string) from an embeded<script>tag towp_localize_script().Updated all
array()s in the file to short syntax ([]). I did this automatically with PHP-CS-Fixer:php-cs-fixer fix modules/custom-status/custom-status.php --rules='{"array_syntax": {"syntax": "short"}}'A lot of whitespace alignment changes were automatically made by my editor.
Steps to Test