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

Enhancement/7251 Restyle AdBlock Warning in Connect Services #8752

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9e61622
Remove disabled styling from connect more services.
jimmymadon May 24, 2024
e82d391
Reorder Modules Setup Warning in connect services.
jimmymadon May 24, 2024
82ca21c
Create a generic version of the AdBlockerWarning component.
jimmymadon May 24, 2024
93c1be0
Move AdBlockerWarningMessage within notifications.
jimmymadon May 24, 2024
5bfbb82
Use the generic version of AdBlockerWarning within ModuleSettingsWarn…
jimmymadon May 24, 2024
0aba91b
Update the default setup incomplete component to disable CTA.
jimmymadon May 24, 2024
233bbed
Remove unnecessary custom setup incomplete component for Ads.
jimmymadon May 24, 2024
a351933
Remove old duplicated AdBlockerWarning for AdSense.
jimmymadon May 24, 2024
f70c06a
Remove old duplicated AdBlockerWarning for Ads.
jimmymadon May 24, 2024
f0eb14b
Move AdBlockerWarningMessage to notifications.
jimmymadon May 24, 2024
927fa3a
Use the common AdBlockerWarning component for Ads and AdSense.
jimmymadon May 24, 2024
3338b58
Refactor tests for common AdBlockerWarinng component.
jimmymadon May 24, 2024
0fc0f92
Reuse generic component for setup errors in AdSense.
jimmymadon May 24, 2024
8bb3d93
Remove incorrect copy-pasted old comment.
jimmymadon May 24, 2024
a0f987a
Update stories to use new common AdBlockerWarning component.
jimmymadon May 24, 2024
2b448b4
Remove duplicated deprecated AdBlockerWarning story.
jimmymadon May 24, 2024
d6b7669
Make storeName optional to fix all storybook stories.
jimmymadon May 27, 2024
284352d
Use string interpolation instead of concatenation.
tofumatt May 27, 2024
100cd86
Fix whitespace issue.
tofumatt May 27, 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
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,30 @@
* limitations under the License.
*/

/**
* External dependencies
*/
import PropTypes from 'prop-types';

/**
* Internal dependencies
*/
import Data from 'googlesitekit-data';

import { MODULES_ADSENSE } from '../../datastore/constants';
import { CORE_SITE } from '../../../../googlesitekit/datastore/site/constants';
import AdBlockerWarningMessage from '../../../../components/AdBlockerWarningMessage';
import { CORE_SITE } from '../../googlesitekit/datastore/site/constants';
import { CORE_MODULES } from '../../googlesitekit/modules/datastore/constants';
import AdBlockerWarningMessage from './AdBlockerWarningMessage';
const { useSelect } = Data;

export default function AdBlockerWarning() {
export default function AdBlockerWarning( { moduleSlug } ) {
const storeName = useSelect( ( select ) =>
select( CORE_MODULES ).getModuleStoreName( moduleSlug )
);
const adBlockerWarningMessage = useSelect( ( select ) =>
select( MODULES_ADSENSE ).getAdBlockerWarningMessage()
select( storeName )?.getAdBlockerWarningMessage()
);
const getHelpLink = useSelect( ( select ) =>
select( CORE_SITE ).getDocumentationLinkURL(
'adsense-ad-blocker-detected'
moduleSlug + '-ad-blocker-detected'
tofumatt marked this conversation as resolved.
Show resolved Hide resolved
)
);

Expand All @@ -43,3 +50,7 @@ export default function AdBlockerWarning() {
/>
);
}

AdBlockerWarning.propTypes = {
moduleSlug: PropTypes.string.isRequired,
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
* Internal dependencies
*/
import AdBlockerWarning from './AdBlockerWarning';
import { render } from '../../../../../../tests/js/test-utils';
import { MODULES_ADSENSE } from '../../datastore/constants';
import { CORE_USER } from '../../../../googlesitekit/datastore/user/constants';
import { provideModules } from '../../../../../../tests/js/utils';
import { render } from '../../../../tests/js/test-utils';
import { MODULES_ADSENSE } from '../../modules/adsense/datastore/constants';
import { CORE_USER } from '../../googlesitekit/datastore/user/constants';
import {
provideModules,
provideModuleRegistrations,
} from '../../../../tests/js/utils';

const setupAdBlockerNotConnectedRegistry = ( registry ) => {
provideModules( registry, [
Expand All @@ -33,6 +36,7 @@ const setupAdBlockerNotConnectedRegistry = ( registry ) => {
connected: false,
},
] );
provideModuleRegistrations( registry );
registry.dispatch( MODULES_ADSENSE ).receiveGetSettings( {} );
registry.dispatch( CORE_USER ).receiveIsAdBlockerActive( true );
};
Expand All @@ -45,20 +49,32 @@ const setupAdBlockerConnectedRegistry = ( registry ) => {
connected: true,
},
] );
provideModuleRegistrations( registry );
registry.dispatch( MODULES_ADSENSE ).receiveGetSettings( {} );
registry.dispatch( CORE_USER ).receiveIsAdBlockerActive( true );
};

const setupNoAdBlockerRegistry = ( registry ) => {
provideModules( registry, [
{
slug: 'adsense',
active: true,
connected: true,
},
] );
provideModuleRegistrations( registry );
registry.dispatch( MODULES_ADSENSE ).receiveGetSettings( {} );
registry.dispatch( CORE_USER ).receiveIsAdBlockerActive( false );
};

describe( 'AdBlockerWarning', () => {
it( 'should render the warning when an AdBlocker is active and module is not connected', () => {
const { container } = render( <AdBlockerWarning />, {
setupRegistry: setupAdBlockerNotConnectedRegistry,
} );
const { container } = render(
<AdBlockerWarning moduleSlug="adsense" />,
{
setupRegistry: setupAdBlockerNotConnectedRegistry,
}
);

expect(
container.querySelector( '.googlesitekit-warning-notice' )
Expand All @@ -68,9 +84,12 @@ describe( 'AdBlockerWarning', () => {
} );

it( 'should render the warning when an AdBlocker is active and module is connected', () => {
const { container } = render( <AdBlockerWarning />, {
setupRegistry: setupAdBlockerConnectedRegistry,
} );
const { container } = render(
<AdBlockerWarning moduleSlug="adsense" />,
{
setupRegistry: setupAdBlockerConnectedRegistry,
}
);

expect(
container.querySelector( '.googlesitekit-warning-notice' )
Expand All @@ -80,9 +99,12 @@ describe( 'AdBlockerWarning', () => {
} );

it( 'should render nothing when no AdBlocker is active', () => {
const { container } = render( <AdBlockerWarning />, {
setupRegistry: setupNoAdBlockerRegistry,
} );
const { container } = render(
<AdBlockerWarning moduleSlug="adsense" />,
{
setupRegistry: setupNoAdBlockerRegistry,
}
);

expect( container.firstChild ).toEqual( null );
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ import { createInterpolateElement } from '@wordpress/element';
/**
* Internal dependencies
*/
import Link from './Link';
import WarningNotice from './WarningNotice';
import ExternalIcon from '../../svg/icons/external-rounded.svg';
import Link from '../Link';
import WarningNotice from '../WarningNotice';
import ExternalIcon from '../../../svg/icons/external-rounded.svg';

export default function AdBlockerWarningMessage( {
getHelpLink = '',
Expand Down
11 changes: 8 additions & 3 deletions assets/js/components/notifications/ModuleSettingsWarning.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@
*/
import Data from 'googlesitekit-data';
import { CORE_MODULES } from '../../googlesitekit/modules/datastore/constants';
import { ERROR_CODE_ADBLOCKER_ACTIVE } from '../../googlesitekit/datastore/user/constants';
import WarningNotice from '../WarningNotice';
import AdBlockerWarning from './AdBlockerWarning';

const { useSelect } = Data;

/*
* A single module. Keeps track of its own active state and settings.
*/
export default function ModuleSettingsWarning( { slug } ) {
const error = useSelect( ( select ) =>
select( CORE_MODULES )?.getCheckRequirementsError( slug )
Expand All @@ -37,5 +36,11 @@ export default function ModuleSettingsWarning( { slug } ) {
return null;
}

// The AdBlockerWarning component also renders the "Get help"
// documentation URL in addition to the error message.
if ( ERROR_CODE_ADBLOCKER_ACTIVE === error.code ) {
return <AdBlockerWarning moduleSlug={ slug } />;
}

return <WarningNotice>{ error.message }</WarningNotice>;
}
12 changes: 11 additions & 1 deletion assets/js/components/settings/DefaultSettingsSetupIncomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,29 @@ export default function DefaultSettingsSetupIncomplete( { slug } ) {
const adminReauthURL = useSelect( ( select ) =>
select( storeName )?.getAdminReauthURL?.()
);
const requirementsError = useSelect( ( select ) =>
select( CORE_MODULES )?.getCheckRequirementsError( slug )
);

return (
<Cell size={ 12 }>
<div className="googlesitekit-settings-module__fields-group googlesitekit-settings-module__fields-group--no-border">
<ModuleSettingsWarning slug={ slug } />
</div>

{ createInterpolateElement(
__(
'Setup incomplete: <a>continue module setup</a>',
'google-site-kit'
),
{
a: <Link href={ adminReauthURL } />,
a: (
<Link
className="googlesitekit-settings-module__edit-button"
href={ adminReauthURL }
disabled={ requirementsError ? true : false }
/>
),
}
) }
</Cell>
Expand Down
10 changes: 3 additions & 7 deletions assets/js/components/settings/SetupModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,7 @@ export default function SetupModule( { slug, name, description } ) {
<div
className={ classnames(
'googlesitekit-settings-connect-module',
`googlesitekit-settings-connect-module--${ slug }`,
{
'googlesitekit-settings-connect-module--disabled':
canActivateModule === false,
}
`googlesitekit-settings-connect-module--${ slug }`
) }
key={ slug }
>
Expand Down Expand Up @@ -126,8 +122,6 @@ export default function SetupModule( { slug, name, description } ) {
{ description }
</p>

<ModuleSettingsWarning slug={ slug } />

<p className="googlesitekit-settings-connect-module__cta">
<Link
onClick={ onSetup }
Expand All @@ -142,6 +136,8 @@ export default function SetupModule( { slug, name, description } ) {
) }
</Link>
</p>

<ModuleSettingsWarning slug={ slug } />
</div>
);
}
Expand Down
43 changes: 0 additions & 43 deletions assets/js/modules/ads/components/common/AdBlockerWarning.js

This file was deleted.

1 change: 0 additions & 1 deletion assets/js/modules/ads/components/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@
* limitations under the License.
*/

export { default as AdBlockerWarning } from './AdBlockerWarning';
export { default as ConversionIDTextField } from './ConversionIDTextField';
export { default as PAXEmbeddedApp } from './PAXEmbeddedApp';
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import whenActive from '../../../../util/when-active';
import whenScopesGranted from '../../../../util/whenScopesGranted';
import { ADWORDS_SCOPE, MODULES_ADS } from '../../datastore/constants';
import PAXEmbeddedApp from '../common/PAXEmbeddedApp';
import { AdBlockerWarning } from '../common';
import AdBlockerWarning from '../../../../components/notifications/AdBlockerWarning';
import { CORE_USER } from '../../../../googlesitekit/datastore/user/constants';
import { CORE_WIDGETS } from '../../../../googlesitekit/widgets/datastore/constants';
const { useSelect } = Data;
Expand Down Expand Up @@ -65,7 +65,7 @@ function PartnerAdsPAXWidget( { WidgetNull, Widget } ) {
if ( isAdblockerActive ) {
return (
<Widget>
<AdBlockerWarning />
<AdBlockerWarning moduleSlug="ads" />
</Widget>
);
}
Expand Down
4 changes: 2 additions & 2 deletions assets/js/modules/ads/components/settings/SettingsEdit.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { MODULES_ADS } from '../../datastore/constants';
import { CORE_USER } from '../../../../googlesitekit/datastore/user/constants';
import SettingsForm from './SettingsForm';
import SettingsView from './SettingsView';
import AdBlockerWarning from '../common/AdBlockerWarning';
import AdBlockerWarning from '../../../../components/notifications/AdBlockerWarning';
import { useFeature } from './../../../../hooks/useFeature';
const { useSelect } = Data;

Expand All @@ -50,7 +50,7 @@ export default function SettingsEdit() {

let viewComponent;
if ( isAdBlockerActive ) {
viewComponent = <AdBlockerWarning />;
viewComponent = <AdBlockerWarning moduleSlug="ads" />;
} else if ( paxEnabled && ( paxConversionID || extCustomerID ) ) {
viewComponent = <SettingsView />;
} else if ( isDoingSubmitChanges ) {
Expand Down

This file was deleted.

Loading
Loading