Disables the AI buttons when the editor or blocks are in HTML mode #21443

mhkuu · 2024-06-18T09:28:51Z

Context

We want to disable the AI buttons when the editor or any of the blocks are in HTML mode.
In this PR, we also fix a small issue with the props of the Toast and Notification in the UI library, that led to a console error.

Summary

This PR can be summarized in the following changelog entry:

Disables the AI buttons when the editor or blocks are in HTML mode.
[@yoast/ui-library 0.0.1] Fixes the autoDismiss prop on the Toast element and Notification component.

Relevant technical choices:

To retrieve and check the editing mode of all blocks, we need a helper function, because getBlocks from Gutenberg only retrieves the top-level blocks (see docs).

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Create a new post in the default editor.
Add various blocks, also create some inner blocks through creating a columns block.
Add some content to these blocks.
Add a keyphrase, make sure it does not appear in the first block (that can be regarded as an introduction).
Confirm that the AI button next to the keyphrase in introduction assessment is present and active.
Switch from visual mode to HTML mode for the editor as a whole -- click the three vertical dots on the top right and then click "Code editor".
Confirm that the AI button next to the keyphrase in introduction assessment is present but disabled.
Switch back to visual mode -- click the three vertical dots on the top right and then click "Visual editor".
Confirm that the AI button next to the keyphrase in introduction assessment is present and active.
For one (or multiple of the) blocks, switch to HTML editing mode -- select a block, click the three vertical dots on the top right of the menu that opens, and then click "Edit as HTML".
Confirm that the AI button next to the keyphrase in introduction assessment is present but disabled.
For all blocks, switch back to visual mode -- select a block in HTML editing mode and click "Edit visually".
Confirm that the AI button next to the keyphrase in introduction assessment is present and active.

Finally, confirm that the (console) error Invalid argument supplied to oneOfType. Expected an array of check functions, but received null at index 1. that is shown without this PR in both the UI and the tests is no longer shown.

Relevant test scenarios

Changes should be tested with the browser console open
Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
Changes should be tested on different browsers
Changes should be tested on multisite

Test instructions for QA when the code is in the RC

QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

This PR affects whether the AI button is enabled, no other impact.

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

I have tested this code to the best of my abilities.
During testing, I had activated all plugins that Yoast SEO provides integrations for.
I have added unit tests to verify the code works as intended.
If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
I have written this PR in accordance with my team's definition of done.
I have checked that the base branch is correctly set.

Innovation

No innovation project is applicable for this PR.
This PR falls under an innovation project. I have attached the innovation label.
I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/plugins-automated-testing/issues/1594

…ks are in visual mode

…sole error

coveralls · 2024-06-18T09:44:50Z

Pull Request Test Coverage Report for Build da23f6869e5c1d62822a62146fd4eeaccad4974a

Details

10 of 11 (90.91%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.009%) to 48.419%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/js/src/helpers/getAllBlocks.js	5	6	83.33%

Totals
Change from base Build 4041cc87ffe9c4b136e5e7a91f3eb4046bb2dad8:	0.009%
Covered Lines:	25094
Relevant Lines:	53689

💛 - Coveralls

leonidasmi

CR: 🏗️

A minor comment about maybe improving the performance of the solution.

Also, it would be nice if we had replication steps for the fix of the console error in the UI library, although not a blocker either.

leonidasmi · 2024-06-19T06:36:13Z

packages/js/src/ai-assessment-fixes/components/ai-assessment-fixes-button.js

@@ -31,6 +32,14 @@ const AIAssessmentFixesButton = ( { id, isPremium } ) => {
 	const focusElementRef = useRef( null );
 	const [ buttonClass, setButtonClass ] = useState( "" );

+	// Only enable the button when the editor is in visual mode and all blocks are in visual mode.
+	const allVisual = useSelect( ( select ) => {


One thing that we can consider for the performance of determining if isEnabled is maybe to

first check the editorMode and return early if it's not on visual and only if it is to check all the blocks.

when checking all blocks, return early when we first see a block mode that's not visual

so that we don't parse through all blocks and inner blocks every time.

But if we think that's a minor improvement and want to be pragmatic, I'm also ok with that 😅

I like the first suggestion, I'll implement that 👍

But every already returns early (see docs) so the second point is not an issue 😄

Resolved by putting the check in a single useSelect call, as also advised by https://developer.wordpress.org/news/2024/03/28/how-to-work-effectively-with-the-useselect-hook/#it-s-more-subtle-when-selecting-from-multiple-stores

…nabled

mhkuu · 2024-06-24T09:35:31Z

Updated the test instructions, also added a small extra unit test, and slightly improved performance. Ready for a new round of CR 😸

coveralls · 2024-06-24T09:44:53Z

Pull Request Test Coverage Report for Build 4911d3c9ba9e1f4ec5ee376d6e35844660cd1bc8

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

12 of 12 (100.0%) changed or added relevant lines in 2 files are covered.
119 unchanged lines in 11 files lost coverage.
Overall coverage increased (+0.1%) to 48.535%

Files with Coverage Reduction	New Missed Lines	%
packages/js/src/redux/reducers/advancedSettings.js	1	80.0%
packages/js/src/initializers/classic-editor-integration.js	3	0.0%
packages/js/src/redux/reducers/preferences.js	3	0.0%
packages/js/src/integrations-page/other-integrations.js	4	0.0%
packages/js/src/components/fills/MetaboxFill.js	5	0.0%
packages/js/src/containers/AdvancedSettings.js	5	0.0%
packages/js/src/helpers/fields/AdvancedFields.js	5	0.0%
packages/js/src/elementor/components/fills/ElementorFill.js	7	0.0%
packages/js/src/elementor.js	19	0.0%
packages/js/src/initializers/block-editor-integration.js	30	0.0%

Totals
Change from base Build 4041cc87ffe9c4b136e5e7a91f3eb4046bb2dad8:	0.1%
Covered Lines:	25094
Relevant Lines:	53587

💛 - Coveralls

leonidasmi

CR + AT is ✅

mhkuu added 2 commits June 18, 2024 10:59

Only enable the button when the editor is in visual mode and all bloc…

6a7368b

…ks are in visual mode

Simplifies the type checking for the autoDismiss prop, resolves a con…

01460ea

…sole error

mhkuu added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Jun 18, 2024

mhkuu requested a review from a team as a code owner June 18, 2024 09:28

mhkuu added this to the feature/fix-assessments milestone Jun 18, 2024

leonidasmi reviewed Jun 19, 2024

View reviewed changes

Yoast deleted a comment from saeed72011 Jun 24, 2024

mhkuu added 2 commits June 24, 2024 11:01

Adds a separate unit test for getAllBlocks

7672ab9

Slightly improves the performance of checking whether the button is e…

867e391

…nabled

leonidasmi approved these changes Jun 25, 2024

View reviewed changes

leonidasmi merged commit bf40f17 into feature/fix-assessments Jun 25, 2024
16 checks passed

leonidasmi deleted the fix/disable-ai-button-when-in-html-mode branch June 25, 2024 10:18

mhkuu mentioned this pull request Jul 16, 2024

Merges the feature branch feature/fix-assessments into trunk #21480

Merged

18 tasks

mhkuu changed the title ~~Disable the AI buttons when the editor or blocks are in HTML mode~~ Disables the AI buttons when the editor or blocks are in HTML mode Jul 16, 2024

mhkuu modified the milestones: feature/fix-assessments, 23.2 Jul 16, 2024

mhkuu added the innovation Innovative issue. Relating to performance, memory or data-flow. label Sep 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disables the AI buttons when the editor or blocks are in HTML mode #21443

Disables the AI buttons when the editor or blocks are in HTML mode #21443

mhkuu commented Jun 18, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

leonidasmi left a comment

leonidasmi Jun 19, 2024

mhkuu Jun 19, 2024 •

edited

Loading

mhkuu Jun 24, 2024

mhkuu commented Jun 24, 2024

coveralls commented Jun 24, 2024 •

edited

Loading

leonidasmi left a comment

Disables the AI buttons when the editor or blocks are in HTML mode #21443

Disables the AI buttons when the editor or blocks are in HTML mode #21443

Conversation

mhkuu commented Jun 18, 2024 • edited Loading

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

coveralls commented Jun 18, 2024 • edited Loading

Pull Request Test Coverage Report for Build da23f6869e5c1d62822a62146fd4eeaccad4974a

Details

💛 - Coveralls

leonidasmi left a comment

Choose a reason for hiding this comment

leonidasmi Jun 19, 2024

Choose a reason for hiding this comment

mhkuu Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

mhkuu Jun 24, 2024

Choose a reason for hiding this comment

mhkuu commented Jun 24, 2024

coveralls commented Jun 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 4911d3c9ba9e1f4ec5ee376d6e35844660cd1bc8

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

leonidasmi left a comment

Choose a reason for hiding this comment

mhkuu commented Jun 18, 2024 •

edited

Loading

coveralls commented Jun 18, 2024 •

edited

Loading

mhkuu Jun 19, 2024 •

edited

Loading

coveralls commented Jun 24, 2024 •

edited

Loading