Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Smart cropping for existing images #252

Merged
merged 4 commits into from
Oct 28, 2020
Merged

Conversation

ShahAaron
Copy link

@ShahAaron ShahAaron commented Oct 27, 2020

Description of the Change

This PR adds the ability to Smart Crop existing images in WP Admin. It relates to #213.

Scope/Approach

  • Adds a button to the attachment page that will initiate the regeneration of thumbnails.

Benefits

Allows Smart cropping of pre-existing images from the media attachments screen.

Verification Process

  • Under ClassifAI -> Image Processing make sure enable smart cropping is unchecked.
  • Add an image to the media gallery
  • Go back to ClassifAI -> Image Processing and set enable smart cropping to checked.
  • Open the existing image in the media attachments screen
  • Press the button to ClassifAI Smart Crop
  • New smart cropped image attachment should be generated

NOTE: I could not check the last step as this needs to be done on a remote server. I was able to track the functionality through the code.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

#213

@ShahAaron
Copy link
Author

@dinhtungdu I cannot check the smart cropping as I needs to be on a remote server. Please advise on how you would like me to proceed.

@jeffpaul jeffpaul added this to the 1.6.0 milestone Oct 27, 2020
@jeffpaul jeffpaul added the type:enhancement New feature or request. label Oct 27, 2020
@dinhtungdu dinhtungdu changed the title Code sniffer fixes. Fix: Smart cropping for existing images Oct 28, 2020
Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is working great on my live server. I have a concern about wording only, otherwise this is ready to be merged.

// Screen returns null on the Media library page.
if ( ! $screen ) {
$alt_tags_text = empty( get_post_meta( $post->ID, '_wp_attachment_image_alt', true ) ) ? __( 'Generate', 'classifai' ) : __( 'Rescan', 'classifai' );
$image_tags_text = empty( wp_get_object_terms( $post->ID, 'classifai-image-tags' ) ) ? __( 'Generate', 'classifai' ) : __( 'Rescan', 'classifai' );
$smart_crop_text = empty( get_transient( 'classifai_azure_computer_vision_smart_cropping_latest_response' ) ) ? __( 'Generate', 'classifai' ) : __( 'Rescan', 'classifai' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$smart_crop_text = empty( get_transient( 'classifai_azure_computer_vision_smart_cropping_latest_response' ) ) ? __( 'Generate', 'classifai' ) : __( 'Rescan', 'classifai' );
$smart_crop_text = empty( get_transient( 'classifai_azure_computer_vision_smart_cropping_latest_response' ) ) ? __( 'Generate', 'classifai' ) : __( 'Regenerate', 'classifai' );

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dinhtungdu I've updated the above wording

@ShahAaron
Copy link
Author

@dinhtungdu I've just noticed I've added this to the media modal:

Screenshot 2020-10-28 at 14 21 47

But not to the media edit page:

Screenshot 2020-10-28 at 14 18 17

I'll get this done also

@ShahAaron
Copy link
Author

@dinhtungdu I've also added the functionality to the media edit page

$settings = get_option( 'classifai_computer_vision' );
$captions = get_post_meta( $post->ID, '_wp_attachment_image_alt', true ) ? __( 'Rescan Alt Text', 'classifai' ) : __( 'Scan Alt Text', 'classifai' );
$tags = ! empty( wp_get_object_terms( $post->ID, 'classifai-image-tags' ) ) ? __( 'Rescan Tags', 'classifai' ) : __( 'Generate Tags', 'classifai' );
$smart_crop = get_transient( 'classifai_azure_computer_vision_smart_cropping_latest_response' ) ? __( 'Regenerate Smart Crop', 'classifai' ) : __( 'Generate Smart Crop', 'classifai' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another wording comment for this, "Smart Crop" is not straightforward for end-users. May be "Generate Smart Thumbnail" or just "Generate Thumbnail"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dinhtungdu changed as requested to "Generate Smart Thumbnail"

Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@jeffpaul jeffpaul merged commit 5d9b6b0 into develop Oct 28, 2020
@jeffpaul jeffpaul deleted the feature/213-crop-images branch October 28, 2020 16:14
@jeffpaul jeffpaul linked an issue Oct 28, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smart Cropping Existing Images in Admin
3 participants