Quick Edit and Bulk edit #350
Conversation
There was a problem hiding this comment.
@dhanendran thanks for the PR.
I have tested and the functionality is working as expected.
However, I just added some code changes below which should be addressed before we proceed with the merge.
|
@faisal-alvi I have addressed your comments. |
inc/classes/class-srm-post-type.php
Outdated
| @@ -746,6 +801,7 @@ public function load_resources() { | |||
| if ( 'redirect_rule' === get_post_type() ) { | |||
| wp_enqueue_style( 'redirectjs', plugin_dir_url( 'safe-redirect-manager/safe-redirect-manager.php' ) . 'assets/css/redirect.css', array(), SRM_VERSION ); | |||
| wp_enqueue_script( 'redirectjs', plugin_dir_url( 'safe-redirect-manager/safe-redirect-manager.php' ) . 'assets/js/redirect.js', array( 'jquery' ), SRM_VERSION ); | |||
| wp_enqueue_script( 'quick-bulk-editjs', plugin_dir_url( 'safe-redirect-manager/safe-redirect-manager.php' ) . 'assets/js/quick-bulk-edit.js', array( 'jquery', 'inline-edit-post' ), SRM_VERSION ); | |||
There was a problem hiding this comment.
@dhanendran, I suggest not using a hard-coded plugin file name. It could cause an issue if the plugin directory name does not match.
cc: @faisal-alvi @dkotter
assets/js/quick-bulk-edit.js
Outdated
| var forceHttps = $('.srm_redirect_rule_force_https', postRow).text(); | ||
|
|
||
| $('select[name="srm_redirect_rule_status_code"]', editRow).val(statusCode); | ||
| if ('✓' === forceHttps) { |
There was a problem hiding this comment.
@dhanendran I think we should not use symbols for comparison.
@dkotter do you have any thoughts?
There was a problem hiding this comment.
Yeah, this seems prone to breaking. Is there anything more specific we could target on, maybe we look for an empty string instead?
There was a problem hiding this comment.
@10up/open-source-practice curious if anyone else has insight on how to handle here?
There was a problem hiding this comment.
@ravinderk @dkotter @jeffpaul I'm using x character for comparison now.
There was a problem hiding this comment.
@dhanendran thanks for the work on this.
- I tested and found the bulk checkbox is not working.
- Can you please also address the code review comments starting from Quick Edit and Bulk edit #350 (comment)?
|
@ravinderk @faisal-alvi I have addressed your comments. @faisal-alvi The bulk select checkbox working fine for me. Can you test this again please? |
|
@dhanendran Thanks for the changes. I retested again in a different setup but the bulk select is not working there too. I am on WP v6.5.4, could you please try to replicate it in that version if you are not already on it?
|
QA Update ❌@dhanendran @faisal-alvi
Testing Environment
|
There was a problem hiding this comment.
@mehul0810 Thanks for the fixes. The bulk select checkbox issue is resolved now. I have a minor "new line removal" suggestion below. Additionally, I'm curious about the necessity of custom JS code for bulk edit and bulk checkbox select functionalities. Why doesn't the WordPress core handle these features by default?
Please also have a look at E2E failures.
|
@faisal-alvi Removed the extra line and added a fix for the E2E tests.
A custom Bulk Edit JS was already there and Bulk Select JS is something I have added after investigating the issue. We are not using an actual custom post type or WP_LIST_TABLE due to which the core functionality for bulk select or bulk edit was not working so we need to add those functionality. Let me know if you have any other questions. Thanks! |
Description of the Change
Added option to Quick edit and Bulk edit redirect's HTTPS status and Force Https meta on the post list table.
Closes #98
How to test the Change
Changelog Entry
Credits
Props @dhanendran
Checklist: