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.

Sign up for GitHub

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

Subscribe block: Hide social followers toggle when showSubscribersTotal is not on or isPublicizeEnabled is false #28944

Merged
merged 10 commits into from
Mar 9, 2023
Merged

Subscribe block: Hide social followers toggle when showSubscribersTotal is not on or isPublicizeEnabled is false #28944

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e2b3533
Don't show "Include social followers in count" when showSubscribersTo…
TimBroddin Feb 14, 2023
a300a83
Don't show "Include social followers in count" when showSubscribersTo…
TimBroddin Feb 14, 2023
ccb0b2a
Merge branch 'trunk' into fix/subscription-block-toggles
TimBroddin Feb 24, 2023
a1e270a
Mock scss imports in jest
glendaviesnz Feb 27, 2023
9a461a3
Fix linting issues
glendaviesnz Feb 27, 2023
b83b623
Mock @wordpress/notices
TimBroddin Feb 27, 2023
566c540
Merge branch 'trunk' into fix/subscription-block-toggles
TimBroddin Mar 1, 2023
f1baa30
Merge branch 'trunk' into fix/subscription-block-toggles
TimBroddin Mar 6, 2023
abf6326
Mock @wordpress/notices in the tests
TimBroddin Mar 6, 2023
2058fe1
Merge remote-tracking branch 'origin/trunk' into fix/subscription-blo…
kraftbj Mar 9, 2023
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
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/subscription-block-social-toggle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: bugfix

Don't show "Include social followers in count" when showSubscribersTotal is not toggled or isPublicizeEnabled is false
22 changes: 14 additions & 8 deletions projects/plugins/jetpack/extensions/blocks/subscriptions/controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { numberFormat } from '@automattic/jetpack-components';
import { usePublicizeConfig } from '@automattic/jetpack-publicize-components';
import { isSimpleSite } from '@automattic/jetpack-shared-extension-utils';
import {
ContrastChecker,
Expand Down Expand Up @@ -54,6 +55,8 @@ export default function SubscriptionControls( {
buttonWidth,
successMessage,
} ) {
const { isPublicizeEnabled } = usePublicizeConfig();

return (
<>
{ isNewsletterFeatureEnabled() && <PaidPlanPanel /> }
Expand Down Expand Up @@ -240,14 +243,17 @@ export default function SubscriptionControls( {
}
} }
/>
<ToggleControl
disabled={ ! showSubscribersTotal }
label={ __( 'Include social followers in count', 'jetpack' ) }
checked={ includeSocialFollowers }
onChange={ () => {
setAttributes( { includeSocialFollowers: ! includeSocialFollowers } );
} }
/>
{ showSubscribersTotal && isPublicizeEnabled ? (
<ToggleControl
disabled={ ! showSubscribersTotal }
label={ __( 'Include social followers in count', 'jetpack' ) }
checked={ includeSocialFollowers }
onChange={ () => {
setAttributes( { includeSocialFollowers: ! includeSocialFollowers } );
} }
/>
) : null }
Comment on lines +246 to +255
Copy link
Member

@manzoorwanijk manzoorwanijk Aug 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work for published posts on simple sites with a free plan because isPublicizeEnabled is more like a local state whether sharing is enabled for the post or not. For published posts, this flag is false if re-sharing is not enabled. For simple sites, resharing is not enabled with free plan.

You can check its source in usePublicizeConfig.

The correct solution here might to check if publicize module is active.

CC: @jeherve

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct solution here might to check if publicize module is active.

This may indeed be a better solution, and we now have useModuleStatus to help with that. @Automattic/zap Is that something you could look into?

Thank you!


<ToggleControl
label={ __( 'Place button on new line', 'jetpack' ) }
checked={ buttonOnNewLine }
Expand Down
2 changes: 2 additions & 0 deletions projects/plugins/jetpack/extensions/blocks/subscriptions/test/controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ jest.mock( '@wordpress/block-editor/build/components/use-setting', () => {
};
} );

jest.mock( '@wordpress/notices', () => {}, { virtual: true } );

const setButtonBackgroundColor = jest.fn();
const setGradient = jest.fn();
const setTextColor = jest.fn();
Expand Down
2 changes: 2 additions & 0 deletions projects/plugins/jetpack/extensions/blocks/subscriptions/test/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ jest.mock( '@wordpress/block-editor', () => ( {
} ),
} ) );

jest.mock( '@wordpress/notices', () => {}, { virtual: true } );

describe( 'SubscriptionEdit', () => {
test( 'adds correct classes to container', async () => {
const { container } = render( <SubscriptionEdit { ...defaultProps } /> );
Expand Down
2 changes: 2 additions & 0 deletions projects/plugins/jetpack/extensions/blocks/subscriptions/test/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ import runBlockFixtureTests from '../../../shared/test/block-fixtures';
// The main block should be the first in the array.
const blocks = [ { name: `jetpack/${ name }`, settings } ];

jest.mock( '@wordpress/notices', () => {}, { virtual: true } );

runBlockFixtureTests( `jetpack/${ name }`, blocks, __dirname );
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/tests/jest.config.extensions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ module.exports = {
...baseConfig.testPathIgnorePatterns,
'extensions/shared/test/block-fixtures.js',
],
moduleNameMapper: {
...baseConfig.moduleNameMapper,
'\\.(css|less|sass|scss)$': '<rootDir>/tests/styles-mock.js',
},
};
1 change: 1 addition & 0 deletions projects/plugins/jetpack/tests/styles-mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {};