-
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4f711d0
Add changelog
Aurorum ec9ffbd
Blog Stats block: Include visitors data
Aurorum 6e4ad75
Remove unneeded class name
Aurorum 54d78c5
Update projects/plugins/jetpack/extensions/blocks/blog-stats/blog-sta…
kangzj 924d9be
Try fix tests
Aurorum 2057abc
Fix translator comment
Aurorum f8a461d
Update translator string
Aurorum 3dbaa4c
Fix comments again
Aurorum 0bb778c
Merge branch 'trunk' into add/blog-stats-visitors
Aurorum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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,11 @@ function BlogStatsEdit( { attributes, className, setAttributes } ) { | |
); | ||
} | ||
|
||
const visitorsPlaceholder = _n( 'visitor', 'visitors', parseInt( stats ), 'jetpack' ); | ||
|
||
/* Translators: Number of views */ | ||
const viewsPlaceholder = _n( 'hit', 'hits', parseInt( stats ), 'jetpack' ); | ||
|
||
return ( | ||
<> | ||
<InspectorControls> | ||
|
@@ -55,13 +71,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 } ) } | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} ); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
.