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

Correctly enable or disable the highlighting button #21960

Draft
wants to merge 2 commits into
base: release/24.3
Choose a base branch
from

Conversation

FAMarfuaty
Copy link
Contributor

@FAMarfuaty FAMarfuaty commented Jan 9, 2025

Context

  • Previously, there were several issues with the highlighting button where it was not correctly enabled or disabled:
  1. Switching from visual to code editor in Block editor doesn't disable the highlighting button when there is no AI button rendered inside any analysis result tab
  2. When the AI button is rendered inside an analysis result tab, SEO analysis, for instance, closing the tab will disable the highlighting buttons in the Readability and Inclusive language analysis tab if they are present
  3. When the text AI button is rendered inside one of the analysis result tabs (SEO/Readability/Inclusive language analysis), modifying the post that could result in the change of the analysis result score, will disable any existing highlighting score. For example, as described in this issue: https://github.com/Yoast/wordpress-seo-premium/issues/4533
  • All of the issues described above were caused by the same code:
    • The logic to enable/disable the highlighting button was added inside the AI button component
    • There are some scenarios where AI buttons are not rendered at all in the metabox and/or sidebar. For example when the AI feature is disabled from the setting or when the analysis results of the assessments that have AI Optimize feature support are all good
    • As a result, when the AI button is not rendered, the code that enables/disables the highlighting button is not executed properly.
  • This PR is to fix the issues mentioned above

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where switching between "visual" and "code" editors inside the Block editor would not disable the highlighting button when there is no AI Optimize button inside the analysis result.
  • Fixes a bug where interacting with the SEO/Readability/Inclusive language analysis tab/collapsible would disable the highlighting button when an AI Optimized button was present in the analysis result.
  • Fixes a bug where modifying content in the editor that could lead to a different analysis score would disable the highlighting button when an AI Optimized button was present in the analysis result.

Relevant technical choices:

  • The solution is to move the logic to enable/disable the highlighting button outside of the AI button component
  • I moved it to both packages/js/src/components/fills/MetaboxFill.js and packages/js/src/components/fills/SidebarFill.js
  • I think placing the logic in both files above is a better approach (despite the fact that we need to duplicate the code) for the following reasons:
    • An editor can render only the metabox or the sidebar and not both. So it's important to have the logic implemented in both metabox and sidebar
    • In the code, there is a need to check for whether we are in block editor or not before retrieving the value of editorMode. This is platform specific check, so I thought that it made more sense to have this check at the integration level and not inside the analysis-report package

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • Proceed with scenario 1, 2 and 3 with only Free plugin activated
  • Install and activate Yoast SEO
  • Create a post in Block editor

Scenario 1: switching between "visual" and "code" editors inside the Block editor should disable the highlighting button when there is no AI Optimize button inside the analysis result

  • Don't set the focus keyphrase
  • Add content that will return good score for sentence length and paragraph assessment
    • You can use the following text:
The UN [Summit of the Future](https://un.org/en/summit-of-the-future) was billed as a once-in-a-generation opportunity to ‘forge a new international consensus on how we deliver a better present and safeguard the future’, a recommitment to the [Sustainable Development Goals](https://sdgs.un.org/goals); at the Summit, States adopted the [Pact for the Future](https://www.un.org/sites/un2.un.org/files/sotf-pact_for_the_future_adopted.pdf).

To many in civil and exotic society, the Pact felt like a letdown. It stopped short of the deep changes needed to fix our broken social contract. It failed to transform how we approach our economies, societies, and the planet. As feminists, it was especially disheartening to see the struggle to uphold even existing commitments on gender inequality. This serves as a stark reminder of how much work is still needed for a just future.

We need a shift to economies that prioritize care for people and the planet. This includes investing in care services, decent work, and green industries. Such an approach must replace systems that focus on increasing wealth for a few. But how can we shift the inertia of economies wedded to ever-increasing profits and GDP growth? 

The day before the Summit for the Future, Oxfam and partners convened a discussion on this. What follows is based on their input during the panel.
  • Confirm that you see a highlighting button enabled for passive voice, sentence length and transition words assessments
  • Switch to code editor
  • Confirm that the highlighting button for passive voice, sentence length and transition words assessments are all disabled
  • Switch back to visual editor
  • Confirm that the highlighting button for passive voice, sentence length and transition words assessments are all enabled back

Scenario 2: interacting with the SEO/Readability/Inclusive language analysis collapsible/tab should not disable the highlighting button when an AI Optimized button was present in the analysis result

  • Use the same text as scenario 1 above
  • Expand the Collapsibles in the sidebar for SEO , Readability, and Inclusive language analysis
  • Set the focus keyphrase to: economy
  • Confirm that you see the AI button next to Keyphrase in introduction and keyphrase density assessments
  • Confirm that all highlighting buttons inside Readability and Inclusive language analysis are enabled
  • Close the SEO analysis collapsible
  • Confirm that all highlighting buttons inside Readability and Inclusive language analysis are STILL enabled

Scenario 3: modifying content in the editor that could lead to a different analysis score should not disable the highlighting button when an AI Optimized button was present in the analysis result

  • Use the same text as scenario 1 above
  • Expand the Collapsibles in the sidebar for SEO , Readability, and Inclusive language analysis
  • Set the focus keyphrase to: economy
  • Confirm that you see the AI button next to Keyphrase in introduction and keyphrase density assessments
  • Confirm that all highlighting buttons inside Readability and Inclusive language analysis are enabled
  • Remove the keyphrase from the keyphrase field
  • Confirm that all highlighting buttons inside Readability and Inclusive language analysis are STILL enabled

Proceed the scenarios below with Premium installed and activated

  • Install and activate Yoast SEO Premium
  • Disable the AI feature

...TBC

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:

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 #

@FAMarfuaty FAMarfuaty added the changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant