-
Notifications
You must be signed in to change notification settings - Fork 800
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
Blog Stats Block: Include Visitors Data #35427
Changes from 6 commits
4f711d0
ec9ffbd
6e4ad75
54d78c5
924d9be
2057abc
f8a461d
3dbaa4c
0bb778c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: enhancement | ||
|
||
Blog Stats Block: allow displaying site's number of visitors. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,10 +11,13 @@ import { InactiveStatsPlaceholder } from './inactive-placeholder'; | |||||||||||||||||||||||||||||||||||||||||||||
function BlogStatsEdit( { attributes, className, setAttributes } ) { | ||||||||||||||||||||||||||||||||||||||||||||||
const { isLoadingModules, isChangingStatus, isModuleActive, changeStatus } = | ||||||||||||||||||||||||||||||||||||||||||||||
useModuleStatus( 'stats' ); | ||||||||||||||||||||||||||||||||||||||||||||||
const { label, statsOption } = attributes; | ||||||||||||||||||||||||||||||||||||||||||||||
const { label, statsData, statsOption } = attributes; | ||||||||||||||||||||||||||||||||||||||||||||||
const [ blogViews, setBlogViews ] = useState( null ); | ||||||||||||||||||||||||||||||||||||||||||||||
const [ blogVisitors, setBlogVisitors ] = useState(); | ||||||||||||||||||||||||||||||||||||||||||||||
const [ postViews, setPostViews ] = useState(); | ||||||||||||||||||||||||||||||||||||||||||||||
const views = statsOption === 'post' ? postViews : blogViews; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const blogStats = statsData === 'views' ? blogViews : blogVisitors; | ||||||||||||||||||||||||||||||||||||||||||||||
const stats = statsOption === 'post' ? postViews : blogStats; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const postId = useSelect( select => select( 'core/editor' ).getCurrentPostId(), [] ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -26,6 +29,7 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) { | |||||||||||||||||||||||||||||||||||||||||||||
: '/wpcom/v2/blog-stats', | ||||||||||||||||||||||||||||||||||||||||||||||
} ).then( response => { | ||||||||||||||||||||||||||||||||||||||||||||||
setBlogViews( response[ 'blog-views' ] ); | ||||||||||||||||||||||||||||||||||||||||||||||
setBlogVisitors( response[ 'blog-visitors' ] ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
// Display "12,345" as an obvious placeholder when we have no Post ID. | ||||||||||||||||||||||||||||||||||||||||||||||
// Applies to widgets, FSE templates etc. | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -34,6 +38,13 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) { | |||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
}, [ postId, isModuleActive ] ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
// We don't collect visitor data for individual posts. | ||||||||||||||||||||||||||||||||||||||||||||||
useEffect( () => { | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||||||||||||||||||||||||||||||||||||||||||||||
if ( statsData === 'visitors' ) { | ||||||||||||||||||||||||||||||||||||||||||||||
setAttributes( { statsOption: 'site' } ); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
}, [ statsData, setAttributes ] ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
if ( ! isModuleActive && ! isLoadingModules ) { | ||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||
<InactiveStatsPlaceholder | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -44,6 +55,12 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) { | |||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/* Translators: Number of visitors */ | ||||||||||||||||||||||||||||||||||||||||||||||
const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 ); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/* Translators: Number of views */ | ||||||||||||||||||||||||||||||||||||||||||||||
const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 ); | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error
is actually coming from here. There's a detailed explanation of what's going on at https://www.npmjs.com/package/@automattic/i18n-check-webpack-plugin#lost-comments-due-to-expression-movement, but the TL;DR is that this code is getting transformed to look something like this instead: /* Translators: Number of visitors */
const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack', 0 ),
viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack', 0 );
/* Translators: Number of views */ (i.e. it deletes the first This seems to work
Suggested change
Or this
Suggested change
I don't know why 924d9be didn't work. Possibly I should remove the corresponding suggestion from the doc I linked if it doesn't work anymore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've moved the comment and added a parameter - hopefully that works! Thanks for taking a look. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, it looks like I committed the wrong thing. 🤦 It works fine locally for me too - re-added your suggestion now! |
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||
<> | ||||||||||||||||||||||||||||||||||||||||||||||
<InspectorControls> | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -55,13 +72,10 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) { | |||||||||||||||||||||||||||||||||||||||||||||
<p className="jetpack-blog-stats__loading">{ __( 'Loading stats…', 'jetpack' ) }</p> | ||||||||||||||||||||||||||||||||||||||||||||||
) : ( | ||||||||||||||||||||||||||||||||||||||||||||||
<p> | ||||||||||||||||||||||||||||||||||||||||||||||
<span>{ numberFormat( views ) } </span> | ||||||||||||||||||||||||||||||||||||||||||||||
<span>{ numberFormat( stats ) } </span> | ||||||||||||||||||||||||||||||||||||||||||||||
<RichText | ||||||||||||||||||||||||||||||||||||||||||||||
tagName="span" | ||||||||||||||||||||||||||||||||||||||||||||||
placeholder={ | ||||||||||||||||||||||||||||||||||||||||||||||
/* Translators: Number of views */ | ||||||||||||||||||||||||||||||||||||||||||||||
_n( 'hit', 'hits', parseInt( views ), 'jetpack' ) | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
placeholder={ statsData === 'visitors' ? visitorsPlaceholder : viewsPlaceholder } | ||||||||||||||||||||||||||||||||||||||||||||||
value={ label } | ||||||||||||||||||||||||||||||||||||||||||||||
allowedFormats={ [ 'core/bold', 'core/italic', 'core/link' ] } | ||||||||||||||||||||||||||||||||||||||||||||||
onChange={ newLabel => setAttributes( { label: newLabel } ) } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import { BlogStatsInspectorControls } from '../controls'; | |
describe( 'BlogStatsControls', () => { | ||
const defaultAttributes = { | ||
label: 'hits', | ||
statsData: 'views', | ||
statsOption: 'site', | ||
}; | ||
|
||
|
@@ -19,10 +20,30 @@ describe( 'BlogStatsControls', () => { | |
} ); | ||
|
||
describe( 'Inspector settings', () => { | ||
test( 'loads and displays settings', () => { | ||
test( 'loads and displays views or visitors settings', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
render( <BlogStatsInspectorControls { ...defaultProps } /> ); | ||
|
||
expect( screen.getByLabelText( 'Views' ) ).toBeInTheDocument(); | ||
expect( screen.getByLabelText( 'Visitors' ) ).toBeInTheDocument(); | ||
} ); | ||
|
||
test( 'defaults stats data to views', () => { | ||
render( <BlogStatsInspectorControls { ...defaultProps } /> ); | ||
|
||
expect( screen.getByLabelText( 'Views' ) ).toBeChecked(); | ||
} ); | ||
|
||
test( 'sets the statsData attribute', async () => { | ||
const user = userEvent.setup(); | ||
render( <BlogStatsInspectorControls { ...defaultProps } /> ); | ||
await user.click( screen.getByLabelText( 'Visitors' ) ); | ||
|
||
expect( setAttributes ).toHaveBeenCalledWith( { statsData: 'visitors' } ); | ||
} ); | ||
|
||
test( 'loads and displays option settings', () => { | ||
render( <BlogStatsInspectorControls { ...defaultProps } /> ); | ||
|
||
expect( screen.getByText( 'Settings' ) ).toBeInTheDocument(); | ||
expect( screen.getByLabelText( 'My whole site' ) ).toBeInTheDocument(); | ||
expect( screen.getByLabelText( 'This individual post' ) ).toBeInTheDocument(); | ||
} ); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try this 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build is still failing 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any insights please @jeherve @anomiex? Thanks
[plugins/jetpack 2:10.354] [build-production-extensions] ERROR in editor-beta.js: Translator comments have gone missing for "hit"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a bit ugly, but I think it relates to where the comment goes: #28351 (comment)
Just added a commit that will maybe fix it? Not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was worth a try, but nope, issue seems to persist!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to work around that by adding a dummy third argument, like it's done here:
jetpack/projects/plugins/jetpack/extensions/blocks/map/styles.js
Lines 22 to 25 in 7693bbd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done! Not sure how to check locally, but hopefully that fixes the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the confusion is that this isn't the code causing that error, it's the JS code in
edit.js
. I'll comment there with details.You should be able to check locally with
jetpack build --production plugins/jetpack
, or inprojects/plugins/jetpack/
runpnpm build-production-extensions
.