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

New general page: updating counters properly #21747

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
11855a2
Add update notification total helper
igorschoester Oct 22, 2024
27d98d4
Refactor the admin bar to always render the notification totals
igorschoester Oct 22, 2024
c89f9cc
Add notification count sync hook
igorschoester Oct 24, 2024
16e67d7
Remove the reindex notification from the FTC
igorschoester Oct 24, 2024
5cb6245
Use named export instead
igorschoester Oct 24, 2024
f4d6063
Improve test coverage
igorschoester Oct 24, 2024
1cfb17d
Fix copy in the Alert center when no problems are detected
pls78 Oct 16, 2024
570d1b3
Update packages/js/src/dashboard/components/problems.js
pls78 Oct 21, 2024
958e42f
Update packages/js/src/dashboard/components/problems.js
pls78 Oct 21, 2024
4b5da38
Update packages/js/src/dashboard/components/problems.js
pls78 Oct 23, 2024
ef890e1
Update packages/js/src/dashboard/components/problems.js
pls78 Oct 23, 2024
7ee252e
Use STORE_NAME constant instead of hard-code it
pls78 Oct 23, 2024
a551b2e
Remove unused localized string
pls78 Oct 23, 2024
7100e3d
Fix cs
pls78 Oct 23, 2024
af9c52d
Remove duplicate line
pls78 Oct 23, 2024
7c12072
Set max width for Paper content.
thijsoo Oct 23, 2024
40b7689
Update snapshots for general notifications and problems
igorschoester Oct 24, 2024
83effcd
Add null coalescing operator to avoid fatal error
pls78 Oct 25, 2024
1d449b5
Removes other uses of matchStringWithRegex
mhkuu Oct 18, 2024
58dbaf2
Convers the text alignment assessment to use the HTML parser
mhkuu Oct 18, 2024
d8b51c0
Further simplifies the getLongCenterAlignedTexts research
mhkuu Oct 18, 2024
54b7c9f
Extracts constants and functions for readability
mhkuu Oct 24, 2024
155e45d
add columns to the semrush api request
vraja-pro Oct 15, 2024
0b8192c
fix test
vraja-pro Oct 15, 2024
dc57cb3
FTC: remove horizontal rule
igorschoester Oct 24, 2024
1356b74
Make indexation step responsive
igorschoester Oct 25, 2024
2f6bfe3
Converts the list assessment to use the HTML parser
mhkuu Oct 21, 2024
b66a78f
Exclude empty list
FAMarfuaty Oct 29, 2024
dcd153e
Add unit tests
FAMarfuaty Oct 29, 2024
3e4eb95
Use native for loop to avoid nested check
FAMarfuaty Oct 29, 2024
8f217a3
Improve check
FAMarfuaty Oct 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions admin/views/partial-notifications-errors.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@
$yoast_seo_dismissed_total = count( $yoast_seo_dismissed );
$yoast_seo_total = $notifications_data['metrics']['errors'];

$yoast_seo_i18n_title = __( 'Problems', 'wordpress-seo' );
$yoast_seo_i18n_issues = __( 'We have detected the following issues that affect the SEO of your site.', 'wordpress-seo' );
$yoast_seo_i18n_no_issues = __( 'Good job! We could detect no serious SEO problems.', 'wordpress-seo' );
$yoast_seo_i18n_muted_issues_title = sprintf(
/* translators: %d expands the amount of hidden problems. */
_n( 'You have %d hidden problem:', 'You have %d hidden problems:', $yoast_seo_dismissed_total, 'wordpress-seo' ),
$yoast_seo_dismissed_total
);
$yoast_seo_i18n_title = __( 'Problems', 'wordpress-seo' );
$yoast_seo_i18n_issues = __( 'We have detected the following issues that affect the SEO of your site.', 'wordpress-seo' );
$yoast_seo_i18n_no_issues = __( 'Good job! We could detect no serious SEO problems.', 'wordpress-seo' );

require WPSEO_PATH . 'admin/views/partial-notifications-template.php';
11 changes: 6 additions & 5 deletions inc/class-wpseo-admin-bar-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -855,14 +855,15 @@ protected function get_notification_counter() {
$notification_center = Yoast_Notification_Center::get();
$notification_count = $notification_center->get_notification_count();

if ( ! $notification_count ) {
return '';
}

/* translators: Hidden accessibility text; %s: number of notifications. */
$counter_screen_reader_text = sprintf( _n( '%s notification', '%s notifications', $notification_count, 'wordpress-seo' ), number_format_i18n( $notification_count ) );

return sprintf( ' <div class="wp-core-ui wp-ui-notification yoast-issue-counter"><span class="yoast-issues-count" aria-hidden="true">%d</span><span class="screen-reader-text">%s</span></div>', $notification_count, $counter_screen_reader_text );
return sprintf(
' <div class="wp-core-ui wp-ui-notification yoast-issue-counter%s"><span class="yoast-issues-count" aria-hidden="true">%d</span><span class="screen-reader-text">%s</span></div>',
( $notification_count ) ? '' : ' yst-hidden',
$notification_count,
$counter_screen_reader_text
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import apiFetch from "@wordpress/api-fetch";
import { useDispatch } from "@wordpress/data";
import { useCallback, useReducer, useState, useEffect } from "@wordpress/element";
import { __ } from "@wordpress/i18n";
import { uniq } from "lodash";

import { STORE_NAME } from "../general/constants";
import { configurationReducer } from "./tailwind-components/helpers/index.js";
import SocialProfilesStep from "./tailwind-components/steps/social-profiles/social-profiles-step";
import Stepper, { Step } from "./tailwind-components/stepper";
Expand Down Expand Up @@ -158,6 +159,7 @@ function calculateInitialState( windowObject, isStepFinished ) {
* @returns {WPElement} The FirstTimeConfigurationSteps component.
*/
export default function FirstTimeConfigurationSteps() {
const { removeAlert } = useDispatch( STORE_NAME );
const [ finishedSteps, setFinishedSteps ] = useState( window.wpseoFirstTimeConfigurationData.finishedSteps );

const isStepFinished = useCallback( ( stepId ) => {
Expand Down Expand Up @@ -192,32 +194,10 @@ export default function FirstTimeConfigurationSteps() {
without triggering a reload, whereas the window variable remains stale. */
useEffect( () => {
if ( indexingState === "completed" ) {
const indexationNotice = document.getElementById( "wpseo-reindex" );
if ( indexationNotice ) {
const allCounters = document.querySelectorAll( ".yoast-issue-counter, #toplevel_page_wpseo_dashboard .update-plugins" );

// Update the notification counters if there are any (non-zero ones).
if ( allCounters.length > 0 && allCounters[ 0 ].firstChild.textContent !== "0" ) {
// Get the oldCount for easier targeting.
const oldCount = allCounters[ 0 ].firstChild.textContent;
const newCount = ( parseInt( oldCount, 10 ) - 1 ).toString();
allCounters.forEach( ( counterNode => {
// The classList replace will return false if the class was not present (and thus an adminbar counter).
const isAdminBarCounter = ! counterNode.classList.replace( "count-" + oldCount, "count-" + newCount );
// If the count reaches zero because of this, remove the red dot alltogether.
if ( isAdminBarCounter && newCount === "0" ) {
counterNode.style.display = "none";
} else {
counterNode.firstChild.textContent = counterNode.firstChild.textContent.replace( oldCount, newCount );
counterNode.lastChild.textContent = counterNode.lastChild.textContent.replace( oldCount, newCount );
}
} ) );
}
indexationNotice.remove();
}
removeAlert( "wpseo-reindex" );
window.yoastIndexingData.amount = "0";
}
}, [ indexingState ] );
}, [ indexingState, removeAlert ] );

const isStep2Finished = isStepFinished( STEPS.siteRepresentation );
const isStep3Finished = isStepFinished( STEPS.socialProfiles );
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { Fragment } from "@wordpress/element";
import { __ } from "@wordpress/i18n";
import PropTypes from "prop-types";
import { ReactComponent as ConfigurationStartImage } from "../../../../../images/indexables_1_left_bubble_optm.svg";
import { FadeInAlert } from "../../base/alert";
import { ConfigurationIndexation } from "./configuration-indexation";

/* eslint-disable complexity */

/**
* The indexation step.
*
Expand All @@ -18,17 +15,17 @@ import { ConfigurationIndexation } from "./configuration-indexation";
* @returns {WPElement} The indexation step.
*/
export default function IndexationStep( { indexingState, setIndexingState, showRunIndexationAlert, isStepperFinished } ) {
return <Fragment>
<div className="yst-flex yst-flex-row yst-justify-between yst-flex-wrap yst-mb-8">
<p className="yst-text-sm yst-whitespace-pre-line yst-w-[463px]">
return <div className="yst-@container">
<div className="yst-flex yst-flex-col @lg:yst-flex-row yst-gap-6 yst-mb-8">
<p className="yst-text-sm yst-whitespace-pre-line">
{ __(
"Let's start by running the SEO data optimization. That means we'll scan your site and create a database with " +
"optimized SEO data. It won't change any content or settings on your site and you don't need to do anything, just hit start!\n" +
"\nNote: If you have a lot of content, this optimization could take a moment. But trust us, it's worth it!",
"wordpress-seo"
) }
</p>
<ConfigurationStartImage className="yst-h-28 yst-w-24 yst-mr-6" />
<ConfigurationStartImage className="yst-shrink-0 yst-h-28 yst-w-24" />
</div>
<div id="yoast-configuration-indexing-container" className="indexation-container">
<ConfigurationIndexation
Expand All @@ -48,7 +45,7 @@ export default function IndexationStep( { indexingState, setIndexingState, showR
"wordpress-seo" )
}
</FadeInAlert>
</Fragment>;
</div>;
}

IndexationStep.propTypes = {
Expand Down
2 changes: 2 additions & 0 deletions packages/js/src/general/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getMigratingNoticeInfo, deleteMigratingNotices } from "../helpers/migra
import Notice from "./components/notice";
import WebinarPromoNotification from "../components/WebinarPromoNotification";
import { shouldShowWebinarPromotionNotificationInDashboard } from "../helpers/shouldShowWebinarPromotionNotification";
import { useNotificationCountSync } from "./hooks/use-notification-count-sync";
import { STEPS as FTC_STEPS } from "../first-time-configuration/constants";

/**
Expand Down Expand Up @@ -79,6 +80,7 @@ const App = () => {
const { pathname } = useLocation();
const alertToggleError = useSelectGeneralPage( "selectAlertToggleError", [], [] );
const { setAlertToggleError } = useDispatch( STORE_NAME );
useNotificationCountSync();

const handleDismiss = useCallback( () => {
setAlertToggleError( null );
Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/general/components/alerts-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Button } from "@yoast/ui-library";
import classNames from "classnames";
import PropTypes from "prop-types";
import { AlertsContext } from "../contexts/alerts-context";
import { STORE_NAME } from "../constants/index";
import { STORE_NAME } from "../constants";

/**
* The alert item object.
Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/general/components/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const Notifications = () => {

return (
<Paper>
<Paper.Content className="yst-flex yst-flex-col yst-gap-y-6">
<Paper.Content className="yst-max-w-[600px] yst-flex yst-flex-col yst-gap-y-6">
<AlertsContext.Provider value={ { ...notificationsTheme } }>
<AlertsTitle counts={ notificationsAlertsList.length } title={ __( "Notifications", "wordpress-seo" ) }>
{ notificationsAlertsList.length === 0 && <p className="yst-mt-2 yst-text-sm">{ __( "No new notifications.", "wordpress-seo" ) }</p> }
Expand Down
10 changes: 8 additions & 2 deletions packages/js/src/general/components/problems.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Paper } from "@yoast/ui-library";
import { AlertsList } from "./alerts-list";
import { AlertsTitle } from "./alerts-title";
import { Collapsible } from "./collapsible";

import { AlertsContext } from "../contexts/alerts-context";
import { STORE_NAME } from "../constants/index";

Expand Down Expand Up @@ -32,10 +33,15 @@ export const Problems = () => {

return (
<Paper>
<Paper.Content className="yst-flex yst-flex-col yst-gap-y-6">
<Paper.Content className="yst-max-w-[600px] yst-flex yst-flex-col yst-gap-y-6">
<AlertsContext.Provider value={ { ...problemsTheme } }>
<AlertsTitle title={ __( "Problems", "wordpress-seo" ) } counts={ problemsList.length }>
<p className="yst-mt-2 yst-text-sm">{ __( "We have detected the following issues that affect the SEO of your site.", "wordpress-seo" ) }</p>
<p className="yst-mt-2 yst-text-sm">
{ problemsList.length > 0
? __( "We have detected the following issues that affect the SEO of your site.", "wordpress-seo" )
: __( "Good job! We could detect no serious SEO problems.", "wordpress-seo" )
}
</p>
</AlertsTitle>
<AlertsList items={ problemsList } />

Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/general/hooks/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@

export { useNotificationCountSync } from "./use-notification-count-sync";
export { default as useSelectGeneralPage } from "./use-select-general-page";
16 changes: 16 additions & 0 deletions packages/js/src/general/hooks/use-notification-count-sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { useSelect } from "@wordpress/data";
import { useEffect } from "@wordpress/element";
import { updateNotificationsCount } from "../../shared-admin/helpers";
import { STORE_NAME } from "../constants";

/**
* Sync the notification count with the Yoast menu and admin bar.
* @returns {void}
*/
export const useNotificationCountSync = () => {
const activeAlertCount = useSelect( ( select ) => select( STORE_NAME ).selectActiveAlertsCount(), [] );

useEffect( () => {
updateNotificationsCount( activeAlertCount );
}, [ activeAlertCount ] );
};
1 change: 0 additions & 1 deletion packages/js/src/general/routes/first-time-configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const FirstTimeConfiguration = () => {
title={ __( "First-time configuration", "wordpress-seo" ) }
description={ __( "Tell us about your site, so we can get it ranked! Let's get your site in tip-top shape for the search engines. Follow these 5 steps to make Google understand what your site is about.", "wordpress-seo" ) }
>
<hr id="configuration-hr-top" />
<div id="yoast-configuration" className="yst-p-8 yst-max-w-[715px]">
<FirstTimeConfigurationSteps />
</div>
Expand Down
51 changes: 38 additions & 13 deletions packages/js/src/general/store/alert-center.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { createSlice, createSelector } from "@reduxjs/toolkit";
import { ASYNC_ACTION_NAMES } from "../../shared-admin/constants";
import { createSelector, createSlice } from "@reduxjs/toolkit";
import { select } from "@wordpress/data";
import { STORE_NAME } from "../constants";
import { get } from "lodash";
import { ASYNC_ACTION_NAMES } from "../../shared-admin/constants";
import { STORE_NAME } from "../constants";

export const ALERT_CENTER_NAME = "alertCenter";

Expand All @@ -14,16 +14,17 @@ const TOGGLE_ALERT_VISIBILITY = "toggleAlertVisibility";
* @param {boolean} hidden The hidden state of the alert.
* @returns {Object} Success or error action object.
*/
export function* toggleAlertStatus( id, nonce, hidden = false ) {
function* toggleAlertStatus( id, nonce, hidden = false ) {
yield{ type: `${ TOGGLE_ALERT_VISIBILITY }/${ ASYNC_ACTION_NAMES.request }` };
try {
yield{ type: TOGGLE_ALERT_VISIBILITY,
yield{
type: TOGGLE_ALERT_VISIBILITY,
payload: {
id,
nonce,
hidden,
},
};
};
return { type: `${ TOGGLE_ALERT_VISIBILITY }/${ ASYNC_ACTION_NAMES.success }`, payload: { id } };
} catch ( error ) {
return { type: `${ TOGGLE_ALERT_VISIBILITY }/${ ASYNC_ACTION_NAMES.error }`, payload: { id } };
Expand Down Expand Up @@ -68,6 +69,14 @@ const slice = createSlice( {
reducers: {
toggleAlert,
setAlertToggleError,
/**
* @param {Object} state The state of the slice.
* @param {string} id The ID of the alert to remove.
* @returns {void}
*/
removeAlert( state, { payload: id } ) {
state.alerts = state.alerts.filter( ( alert ) => alert.id !== id );
},
},
extraReducers: ( builder ) => {
builder.addCase( `${ TOGGLE_ALERT_VISIBILITY }/${ ASYNC_ACTION_NAMES.success }`, ( state, { payload: { id } } ) => {
Expand All @@ -90,34 +99,50 @@ export const getInitialAlertCenterState = slice.getInitialState;
* @param {object} state The state.
* @returns {array} The alerts.
*/
const selectAlerts = ( state ) => get( state, `${ALERT_CENTER_NAME}.alerts`, [] );
const selectAlerts = ( state ) => get( state, `${ ALERT_CENTER_NAME }.alerts`, [] );

const selectActiveAlerts = createSelector(
[ selectAlerts ],
( alerts ) => alerts.filter( ( alert ) => ! alert.dismissed )
);

/**
* Selector to get the alert toggle error.
*
* @param {object} state The state.
* @returns {string} id The id of the alert which caused the error..
*/
const selectAlertToggleError = ( state ) => get( state, `${ALERT_CENTER_NAME}.alertToggleError`, null );
const selectAlertToggleError = ( state ) => get( state, `${ ALERT_CENTER_NAME }.alertToggleError`, null );

export const alertCenterSelectors = {
selectActiveProblems: createSelector(
[ selectAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "error" && ! alert.dismissed )
[ selectActiveAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "error" )
),
selectDismissedProblems: createSelector(
[ selectAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "error" && alert.dismissed )
),
selectActiveNotifications: createSelector(
[ selectAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "warning" && ! alert.dismissed )
[ selectActiveAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "warning" )
),
selectDismissedNotifications: createSelector(
[ selectAlerts ],
( alerts ) => alerts.filter( ( alert ) => alert.type === "warning" && alert.dismissed )
),
selectAlertToggleError,
selectAlert: createSelector(
[
selectAlerts,
( state, id ) => id,
],
( alerts, id ) => alerts.find( ( alert ) => alert.id === id )
),
selectActiveAlertsCount: createSelector(
[ selectActiveAlerts ],
( alerts ) => alerts.length
),
};

export const alertCenterActions = {
Expand Down Expand Up @@ -148,4 +173,4 @@ export const alertCenterControls = {
},
};

export default slice.reducer;
export const alertCenterReducer = slice.reducer;
4 changes: 2 additions & 2 deletions packages/js/src/general/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { STORE_NAME } from "../constants";
import preferences, { createInitialPreferencesState, preferencesActions, preferencesSelectors } from "./preferences";
import { reducers, selectors, actions } from "@yoast/externals/redux";
import * as dismissedAlertsControls from "../../redux/controls/dismissedAlerts";
import alertCenter, { alertCenterActions, alertCenterSelectors, getInitialAlertCenterState, alertCenterControls, ALERT_CENTER_NAME } from "./alert-center";
import { alertCenterReducer, alertCenterActions, alertCenterSelectors, getInitialAlertCenterState, alertCenterControls, ALERT_CENTER_NAME } from "./alert-center";

const { currentPromotions, dismissedAlerts, isPremium } = reducers;
const { isAlertDismissed, getIsPremium, isPromotionActive } = selectors;
Expand Down Expand Up @@ -50,7 +50,7 @@ const createStore = ( { initialState } ) => {
reducer: combineReducers( {
[ LINK_PARAMS_NAME ]: linkParamsReducer,
preferences,
[ ALERT_CENTER_NAME ]: alertCenter,
[ ALERT_CENTER_NAME ]: alertCenterReducer,
currentPromotions,
dismissedAlerts,
isPremium,
Expand Down
1 change: 1 addition & 0 deletions packages/js/src/shared-admin/helpers/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export { fixWordPressMenuScrolling } from "./fix-wordpress-menu-scrolling";
export { updateNotificationsCount } from "./notifications-count";
41 changes: 41 additions & 0 deletions packages/js/src/shared-admin/helpers/notifications-count.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { _n, sprintf } from "@wordpress/i18n";

/**
* Update the text content of an element if it exists.
* @param {HTMLElement} root The root element.
* @param {string} selector The selector.
* @param {string} text The text.
* @returns {HTMLElement|null} The element or null.
*/
const updateTextContentIfElementExists = ( root, selector, text ) => {
const element = root.querySelector( selector );
if ( element ) {
element.textContent = text;
}
return element;
};

/**
* Update the notification total in the Yoast SEO menu and the admin bar badges.
* @param {number} total The total number of notifications.
* @returns {void}
*/
export const updateNotificationsCount = ( total ) => {
// Note: these translation is the same as on the server side.
/* translators: Hidden accessibility text; %s: number of notifications. */
const screenReaderText = sprintf( _n( "%s notification", "%s notifications", total, "wordpress-seo" ), total );

const menuItems = document.querySelectorAll( "#toplevel_page_wpseo_dashboard .update-plugins" );
for ( const menuItem of menuItems ) {
menuItem.className = `update-plugins count-${ total }`;
updateTextContentIfElementExists( menuItem, ".plugin-count", String( total ) );
updateTextContentIfElementExists( menuItem, ".screen-reader-text", screenReaderText );
}

const adminBarItems = document.querySelectorAll( "#wp-admin-bar-wpseo-menu .yoast-issue-counter" );
for ( const adminBar of adminBarItems ) {
adminBar.classList.toggle( "yst-hidden", total === 0 );
updateTextContentIfElementExists( adminBar, ".yoast-issues-count", String( total ) );
updateTextContentIfElementExists( adminBar, ".screen-reader-text", screenReaderText );
}
};
Loading