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

Jump to bottom

fix: fallback if no newspack plugin #1179

Merged
merged 6 commits into from
Aug 15, 2023
Merged

fix: fallback if no newspack plugin #1179

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ffbf39e
fix: allow prompts to be displayed without segmentation if no Newspac…
dkoo Aug 8, 2023
d4b9a25
refactor: avoid use of negative variable name
dkoo Aug 9, 2023
ef8ac67
fix: newspack_popups_view.segments is not an array
dkoo Aug 9, 2023
873a02b
Merge branch 'epic/rearchitecture' into fix/fallback-if-no-newspack-p…
leogermani Aug 11, 2023
b42006b
fix: hide segments taxonomy
leogermani Aug 11, 2023
5b76d36
Merge branch 'epic/rearchitecture' into fix/fallback-if-no-newspack-p…
Aug 14, 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
34 changes: 20 additions & 14 deletions includes/class-newspack-popups-inserter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,23 +658,29 @@ public static function enqueue_scripts() {
true
);

$segments = Newspack_Popups_Segmentation::get_segments( false );

// Gather segments for all prompts to be displayed.
foreach ( $segments as $segment ) {
if ( ! empty( $segment ) && ! empty( $segment['criteria'] ) && ! isset( self::$segments[ $segment['id'] ] ) ) {
self::$segments[ $segment['id'] ] = [
'criteria' => $segment['criteria'],
'priority' => $segment['priority'],
];
}
}

$script_data = [
'debug' => defined( 'WP_DEBUG' ) && WP_DEBUG,
'segments' => self::$segments,
'debug' => defined( 'WP_DEBUG' ) && WP_DEBUG,
];

if ( class_exists( '\Newspack\Reader_Data' ) ) {
$segments = Newspack_Popups_Segmentation::get_segments( false );

// Gather segments for all prompts to be displayed.
foreach ( $segments as $segment ) {
if ( ! empty( $segment ) && ! empty( $segment['criteria'] ) && ! isset( self::$segments[ $segment['id'] ] ) ) {
self::$segments[ $segment['id'] ] = [
'criteria' => $segment['criteria'],
'priority' => $segment['priority'],
];
}
}

$script_data['segments'] = self::$segments;

} else {
$script_data['segmentation_disabled'] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we avoid the negative name here? It can become very confusing (as we saw yesterday)

Copy link
Contributor Author

@dkoo dkoo Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Good call. d4b9a25 removes it, as there's really no need for a separate variable at all. More simply, if there are no segments to handle, then there's no need to use the reader data library at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, ef8ac67 fixes a bug in the prior commit which assumed that newspack_popups_view.segments would be an array and therefore have a length property, when in fact it's a keyed object.

}

\wp_localize_script( $script_handle, 'newspack_popups_view', $script_data );
\wp_enqueue_script( $script_handle );
}
Expand Down
1 change: 1 addition & 0 deletions includes/class-newspack-popups.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ public static function enqueue_block_editor_assets() {
)
),
'preview_query_keys' => self::PREVIEW_QUERY_KEYS,
'segmentation_enabled' => class_exists( '\Newspack\Reader_Data' ),
]
);
\wp_enqueue_style(
Expand Down
46 changes: 24 additions & 22 deletions src/editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,29 +80,31 @@ registerPlugin( 'newspack-popups', {
icon: null,
} );

registerPlugin( 'newspack-popups-frequency', {
render: () => (
<PluginDocumentSettingPanel
name="-frequency-panel"
title={ __( 'Frequency', 'newspack-popups' ) }
>
<FrequencySidebarWithData />
</PluginDocumentSettingPanel>
),
icon: null,
} );
if ( window?.newspack_popups_data?.segmentation_enabled ) {
registerPlugin( 'newspack-popups-frequency', {
render: () => (
<PluginDocumentSettingPanel
name="-frequency-panel"
title={ __( 'Frequency', 'newspack-popups' ) }
>
<FrequencySidebarWithData />
</PluginDocumentSettingPanel>
),
icon: null,
} );

registerPlugin( 'newspack-popups-segmentation', {
render: () => (
<PluginDocumentSettingPanel
name="popup-segmentation-panel"
title={ __( 'Segmentation', 'newspack-popups' ) }
>
<SegmentationSidebarWithData />
</PluginDocumentSettingPanel>
),
icon: null,
} );
registerPlugin( 'newspack-popups-segmentation', {
render: () => (
<PluginDocumentSettingPanel
name="popup-segmentation-panel"
title={ __( 'Segmentation', 'newspack-popups' ) }
>
<SegmentationSidebarWithData />
</PluginDocumentSettingPanel>
),
icon: null,
} );
}

registerPlugin( 'newspack-popups-colors', {
render: () => (
Expand Down
20 changes: 15 additions & 5 deletions src/view/segmentation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ import {
* Match reader to segments.
*/
export const handleSegmentation = prompts => {
window.newspackRAS = window.newspackRAS || [];
window.newspackRAS.push( ras => {
const maybeDisplayPrompts = ( ras = null ) => {
const segments = newspack_popups_view?.segments || {};
const matchingSegment = getBestPrioritySegment( segments );
debug( 'matchingSegment', matchingSegment );
// Log a pageview for frequency counts.
logPageview( ras );
if ( ras ) {
logPageview( ras );
}
let overlayDisplayed;

prompts.forEach( prompt => {
Expand Down Expand Up @@ -54,7 +55,9 @@ export const handleSegmentation = prompts => {
prompt.classList.remove( 'hidden' );

// Log a "prompt_seen" activity when the prompt becomes visible.
handleSeen( prompt, ras );
if ( ras ) {
handleSeen( prompt, ras );
}
};
if ( isOverlay ) {
const scroll = prompt.getAttribute( 'data-scroll' );
Expand All @@ -77,5 +80,12 @@ export const handleSegmentation = prompts => {
// Debug logging for prompt display.
debug( promptId, shouldDisplay );
} );
} );
};

if ( newspack_popups_view.segmentation_disabled ) {
maybeDisplayPrompts();
} else {
window.newspackRAS = window.newspackRAS || [];
window.newspackRAS.push( maybeDisplayPrompts );
}
};
63 changes: 33 additions & 30 deletions src/view/utils/segments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,45 +70,48 @@ export const shouldPromptBeDisplayed = ( prompt, matchingSegment, ras, override
return override;
}

// By frequency.
// eslint-disable-next-line @wordpress/no-unused-vars-before-return
const [ start, between, max, reset ] = prompt.getAttribute( 'data-frequency' ).split( ',' );
const pageviews = ras.store.get( 'pageviews' );
if ( pageviews[ reset ] ) {
const views = pageviews[ reset ].count || 0;
if ( ras ) {
// By frequency.
// eslint-disable-next-line @wordpress/no-unused-vars-before-return
const [ start, between, max, reset ] = prompt.getAttribute( 'data-frequency' ).split( ',' );
const pageviews = ras.store.get( 'pageviews' );
if ( pageviews[ reset ] ) {
const views = pageviews[ reset ].count || 0;

// If reader hasn't amassed enough pageviews yet.
if ( views <= parseInt( start ) ) {
return false;
}
// If reader hasn't amassed enough pageviews yet.
if ( views <= parseInt( start ) ) {
return false;
}

// If not displaying every pageview.
if ( 0 < between ) {
const viewsAfterStart = Math.max( 0, views - ( parseInt( start ) + 1 ) );
if ( 0 < viewsAfterStart % ( parseInt( between ) + 1 ) ) {
// If not displaying every pageview.
if ( 0 < between ) {
const viewsAfterStart = Math.max( 0, views - ( parseInt( start ) + 1 ) );
if ( 0 < viewsAfterStart % ( parseInt( between ) + 1 ) ) {
return false;
}
}

// If there's a max frequency.
const promptId = getRawId( prompt.getAttribute( 'id' ) );
const seenEvents = ( ras.getActivities( 'prompt_seen' ) || [] ).filter( activity => {
return (
activity.data?.prompt_id === promptId &&
periods[ reset ] > Date.now() - activity.timestamp
);
} );
if ( 0 < parseInt( max ) && seenEvents.length >= parseInt( max ) ) {
return false;
}
}

// If there's a max frequency.
const promptId = getRawId( prompt.getAttribute( 'id' ) );
const seenEvents = ( ras.getActivities( 'prompt_seen' ) || [] ).filter( activity => {
return (
activity.data?.prompt_id === promptId && periods[ reset ] > Date.now() - activity.timestamp
);
} );
if ( 0 < parseInt( max ) && seenEvents.length >= parseInt( max ) ) {
// By assigned segments.
const assignedSegments = prompt.getAttribute( 'data-segments' )
? prompt.getAttribute( 'data-segments' ).split( ',' )
: null;
if ( assignedSegments && 0 > assignedSegments.indexOf( matchingSegment ) ) {
return false;
}
}

// By assigned segments.
const assignedSegments = prompt.getAttribute( 'data-segments' )
? prompt.getAttribute( 'data-segments' ).split( ',' )
: null;
if ( assignedSegments && 0 > assignedSegments.indexOf( matchingSegment ) ) {
return false;
}

return true;
};