feat/278: Add settings fields to enable storing image captions in non-alt text image fields. #374

Sidsector9 · 2022-09-30T07:08:16Z

Description of the Change

This PR adds settings to add image captions to the following places:

Closes #278

How to test the Change

Test for a new installation.
Test for old installation with previous setup.

Changelog Entry

Added - Ability to set alt text as image caption and image description.

Credits

Props @jeffpaul @Sidsector9

Checklist:

I agree to follow this project's Code of Conduct.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests pass.

jeffpaul · 2022-11-10T15:49:47Z

@Sidsector9 is there more work needed here or can we assign OSP to get this into review?

Sidsector9 · 2022-11-10T16:16:35Z

@jeffpaul few minor things I need to take care of. I'll prepare the final PR next week.

peterwilsoncc

I've added a couple of notes inline, there's a bug for sites with data saved in the old format.

It would be good to add a test for the old format option of enable_image_captions in computer vision phpunit tests. I think it would start with the following:

	public function test_option_reformatting() {
		add_filter(
			'pre_get_option_classifai_computer_vision',
			function() {
				return array (
					'valid' => false,
					'url' => '',
					'api_key' => '',
					'enable_image_captions' => '1',
					'enable_image_tagging' => '1',
					'enable_smart_cropping' => 'no',
					'enable_ocr' => 'no',
					'enable_read_pdf' => 'no',
					'caption_threshold' => 75,
					'tag_threshold' => 70,
					'image_tag_taxonomy' => 'classifai-image-tags',
				);
			}
		);

		// Get the alt text formatting.
		$settings = $this->get_computer_vision()->get_alt_text_settings();

		// Some assertion to confirm ALT text is enabled.
		// Some assertion to confirm the data is an array.
		// Some assertion to confirm the defaults are as expected for the two new settings.
	}

peterwilsoncc · 2022-11-28T03:17:19Z

includes/Classifai/Services/ImageProcessing.php

+		wp_localize_script(
+			'classifai-media-script',
+			'classifaiMediaVars',
+			array(
+				'enabledAltTextFields' => $this->provider_classes[0]->get_alt_text_settings() ? $this->provider_classes[0]->get_alt_text_settings() : array(),
+			)
+		);


For settings, it's recommended to use wp_add_inline_script() instead.

The method wp_localize_script() uses does some type casting so it's not guaranteed that the data will always be in the form expected.

includes/Classifai/Providers/Azure/ComputerVision.php

peterwilsoncc · 2022-11-28T04:15:34Z

Sorry, noticed some additional errors:

I was sometimes getting a warning too:

Invalid argument supplied for foreach()
    content/plugins/classifai/includes/Classifai/Providers/Azure/ComputerVision.php:84
    Classifai\P\A\ComputerVision->get_alt_text_settings()
    content/plugins/classifai/includes/Classifai/Services/ImageProcessing.php:47
    Classifai\Services\ImageProcessing->enqueue_media_scripts()
    wp-includes/class-wp-hook.php:308
    do_action('admin_enqueue_scripts')
    wp-admin/admin-header.php:118

Generate alt text no longer seems appropriate here. "Generate descriptive text" could be better. This applies to the threshold option too.

peterwilsoncc

Added a few notes below. I'm still seeing notices and warnings under certain circumstances that it would be good to figure out.

I think a few additional isset checks are needed, or always using the new method when it's possible. Using the new method may not be possible if you need to account for different ai providers though.

peterwilsoncc · 2022-12-01T00:07:51Z

tests/Classifai/Providers/Azure/ComputerVisionTest.php

+			'image_tag_taxonomy'    => 'classifai-image-tags',
+		);
+
+		/** Test with `enable_image_captions` set to `1` */


Suggested change

/** Test with `enable_image_captions` set to `1` */

// Test with `enable_image_captions` set to `1`

A couple of things here:

/** indicated a docblock rather than a comment, however....

the WPCS for single line comments is to use // rather than /*

peterwilsoncc · 2022-12-01T00:08:10Z

tests/Classifai/Providers/Azure/ComputerVisionTest.php

+			)
+		);
+
+		/** Test with `enable_image_captions` set to `no` */


Suggested change

/** Test with `enable_image_captions` set to `no` */

// Test with `enable_image_captions` set to `no`

peterwilsoncc · 2022-12-01T00:37:08Z

includes/Classifai/Providers/Azure/ComputerVision.php

+		$enabled_fields = array();
+
+		if ( ! isset( $settings['enable_image_captions'] ) ) {
+			return false;


Suggested change

return false;

return array();

I noticed that when no fields are set, this method sometimes returns false and sometimes an empty array. It would be good to be consistent.

This might allow you to simplify generate_alt_tags() a little as the method could assume an array and lose the repeated is_array() check.

To reproduce:

delete the option from the database

var_dump shows false

save the image processing options page without enabling the descriptive text for any option

var_dump of this method shows array()

peterwilsoncc · 2022-12-01T00:49:42Z

includes/Classifai/Providers/Provider.php

+			if ( ! is_array( $setting_index['enable_image_captions'] ) ) {
+				$default_value = '1' === $setting_index['enable_image_captions'] ? 'alt' : '';


These two lines throw a notice if you visit the page before the options are saved. To reproduce:

Delete the classifai_computer_vision option from the database

Visit the options page

A notice will be thrown

Saving the option then caused different errors, continue from above:

save the options without making any changes

notices are thrown on line 233 for each item in the loop:

Undefined index: alt

Undefined index: caption

Undefined index: description

peterwilsoncc

A couple more notes inline.

PHP notices thrown in previous testing have been resolved, thank you! It would be good to add a test for the deleted option as that was causing problems earlier.

peterwilsoncc · 2023-01-02T22:02:32Z

includes/Classifai/Providers/Azure/ComputerVision.php

@@ -770,8 +827,8 @@ public function setup_fields_sections() {
 		);
 		add_settings_field(
 			'enable-image-captions',
-			esc_html__( 'Generate alt text', 'classifai' ),
-			[ $this, 'render_input' ],
+			esc_html__( 'Generate descriptive text', 'classifai' ),


Alt text confidence threshold will need to become Descriptive text confidence threshold in the next option.

peterwilsoncc · 2023-01-02T22:12:18Z

.eslintrc.json

-		"FormData": "readonly"
+		"FormData": "readonly",
+		"classifaiMediaVars": "readonly"


classifaiMediaVars was removed in 1bfe51c

@peterwilsoncc the PR wasn't up for re-review. I re-added classifaiMediaVars in ba3162a

peterwilsoncc

The code all looks good but one thing I noticed is that the feature branch no longer automatically adds the alt text when an image is uploaded via the block editor

I tested this after saving the image processing page using the new settings.

Develop branch

Feature branch

Sidsector9 · 2023-01-26T08:47:23Z

@peterwilsoncc The descriptive texts are generating for me in the block editor.

I tested drag and drop as well as uploading via the image block using the upload button.

Please watch this screencast:

classifai-278.mov

peterwilsoncc

@Sidsector9 Sorry, not sure what went wrong last time. It seems to be working now.

Possibly I confused WP / the bot by using ngrok -- setting the ngrok tunnel's domain as the site's primary domain worked. I'll keep that in mind for next time.

…as appropriate. While fixing that, found a few bugs that were introduced as part of #374, so fixed those up

Sidsector9 added 5 commits August 22, 2022 19:17

fix: check active tab

4f8e604

Merge branch 'develop' of github.com:10up/classifai into develop

d6a383b

add optional fields to store image captions

db86975

fix formatting

c25cbc9

fix: image caption field texts

3ab0761

Sidsector9 self-assigned this Oct 30, 2022

jeffpaul added this to the 1.9.0 milestone Nov 10, 2022

Sidsector9 added 6 commits November 27, 2022 20:16

fix eslint errors

944a76e

fix min PHP version in workflow

30bf056

Merge branch 'develop' of github.com:10up/classifai into develop

a2ef276

Merge branch 'develop' into feat/278

524bfd3

fix image processing test case

7d26bc4

fix phpunit tests

2135dcc

Sidsector9 requested review from a team and peterwilsoncc and removed request for a team November 27, 2022 18:32

peterwilsoncc requested changes Nov 28, 2022

View reviewed changes

Sidsector9 added 4 commits November 28, 2022 17:31

fix option value when swithching branch

33e7665

align with original functionality

1bfe51c

fix setting labels

5bae591

add phpunit tests

d17e6f5

Sidsector9 force-pushed the feat/278 branch from 797af1b to d17e6f5 Compare November 28, 2022 14:00

Sidsector9 requested a review from peterwilsoncc November 30, 2022 17:20

peterwilsoncc reviewed Dec 1, 2022

View reviewed changes

Sidsector9 added 3 commits January 2, 2023 19:28

fix comments

72de876

return array as default value

9d76d35

Merge branch 'develop' into feat/278

78d531a

peterwilsoncc reviewed Jan 2, 2023

View reviewed changes

fix JS

831ba19

Sidsector9 force-pushed the feat/278 branch from ba3162a to 831ba19 Compare January 3, 2023 02:00

Sidsector9 added 5 commits January 20, 2023 12:46

rename alt text to geenrative text

91c9308

Merge branch 'develop' into feat/278

a9c3c73

Merge branch 'feat/278' of github.com:10up/classifai into feat/278

c648613

add test for missing classifai_computer_vision option

5982517

replace wp_localize_script

e2bf5b8

Sidsector9 force-pushed the feat/278 branch from 65d981b to e2bf5b8 Compare January 20, 2023 13:39

Sidsector9 requested a review from peterwilsoncc January 23, 2023 06:32

replace alt text with descriptive text

b88f0b2

Sidsector9 marked this pull request as ready for review January 24, 2023 05:35

Sidsector9 requested review from a team, dkotter and jeffpaul as code owners January 24, 2023 05:35

peterwilsoncc reviewed Jan 25, 2023

View reviewed changes

Merge branch 'develop' into feat/278

2d777c7

jeffpaul requested review from peterwilsoncc and removed request for jeffpaul January 30, 2023 14:20

peterwilsoncc approved these changes Jan 31, 2023

View reviewed changes

jeffpaul merged commit 117b49c into develop Feb 20, 2023

jeffpaul deleted the feat/278 branch February 20, 2023 21:08

dkotter added a commit that referenced this pull request May 1, 2023

Make sure we show the Scan Image option in the bulk actions dropdown …

62ab217

…as appropriate. While fixing that, found a few bugs that were introduced as part of #374, so fixed those up

dkotter mentioned this pull request Mar 6, 2024

Ensure both arguments passed to merge_settings are an array #733

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/278: Add settings fields to enable storing image captions in non-alt text image fields. #374

feat/278: Add settings fields to enable storing image captions in non-alt text image fields. #374

Sidsector9 commented Sep 30, 2022 •

edited

Loading

jeffpaul commented Nov 10, 2022

Sidsector9 commented Nov 10, 2022

peterwilsoncc left a comment

peterwilsoncc Nov 28, 2022

peterwilsoncc commented Nov 28, 2022

peterwilsoncc left a comment

peterwilsoncc Dec 1, 2022

peterwilsoncc Dec 1, 2022

peterwilsoncc Dec 1, 2022

peterwilsoncc Dec 1, 2022

peterwilsoncc left a comment

peterwilsoncc Jan 2, 2023

peterwilsoncc Jan 2, 2023

Sidsector9 Jan 3, 2023

peterwilsoncc left a comment

Sidsector9 commented Jan 26, 2023

peterwilsoncc left a comment

	/** Test with `enable_image_captions` set to `1` */
	// Test with `enable_image_captions` set to `1`

		if ( ! is_array( $setting_index['enable_image_captions'] ) ) {
		$default_value = '1' === $setting_index['enable_image_captions'] ? 'alt' : '';

feat/278: Add settings fields to enable storing image captions in non-alt text image fields. #374

feat/278: Add settings fields to enable storing image captions in non-alt text image fields. #374

Conversation

Sidsector9 commented Sep 30, 2022 • edited Loading

Description of the Change

How to test the Change

Changelog Entry

Credits

Checklist:

jeffpaul commented Nov 10, 2022

Sidsector9 commented Nov 10, 2022

peterwilsoncc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc commented Nov 28, 2022

peterwilsoncc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

Sidsector9 commented Jan 26, 2023

peterwilsoncc left a comment

Choose a reason for hiding this comment

Sidsector9 commented Sep 30, 2022 •

edited

Loading