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

Toggle Distributor controls when post status changes #1022

Merged
merged 9 commits into from
Mar 8, 2023
+36 −8
Merged

Toggle Distributor controls when post status changes #1022

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fd7761a
Toggle Distributor controls when post status changes
ggutenberg Mar 2, 2023
f384c5d
Address PR comments
ggutenberg Mar 3, 2023
8a66886
Update post title in top menu when it changes
ggutenberg Mar 3, 2023
3367955
Include the original post title when loading pages.
peterwilsoncc Mar 6, 2023
f538b9d
Address PR comments
ggutenberg Mar 6, 2023
e66b2f8
Make dt a dependency of dtGutenberg
ggutenberg Mar 7, 2023
b0aa889
Merge branch 'develop' into feature/toggle-controls-on-status-change
ggutenberg Mar 7, 2023
d9f355a
Add test to ensure the button distributor panel toggles on publish.
peterwilsoncc Mar 8, 2023
9031503
Revert "Add test to ensure the button distributor panel toggles on pu…
peterwilsoncc Mar 8, 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 assets/css/push.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ $mediaSmallScreen : "min-width: 480px";
$mediaMediumScreen : "min-width: 768px";
$mediaGutenbergSmallScreen : "max-width: 782px";

#wp-admin-bar-distributor.hide {
display: none;
}

#distributor-push-wrapper,
#wpadminbar #wp-admin-bar-distributor-placeholder > .ab-item {
display: none;
Expand Down
13 changes: 13 additions & 0 deletions assets/js/gutenberg-plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,23 @@ const DistributorPlugin = () => {
return null;
}

const distributorTopMenu = document.querySelector(
'#wp-admin-bar-distributor'
);

// eslint-disable-next-line no-shadow, react-hooks/rules-of-hooks -- permission checks are needed.
const post = useSelect( ( select ) =>
select( 'core/editor' ).getCurrentPost()
);
// Make the post title available to the top menu.
dtGutenberg.postTitle = post.title;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
dtGutenberg.postTitle = post.title;
dt.postTitle = post.title;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is strange. If I modify the const { document, dtGutenberg, MouseEvent } = window; line and change it to const { document, dtGutenberg, MouseEvent, dt } = window;, dt is undefined. However if I don't make that modification I'm able to reference it in code directly via dt.postTitle or window.dt.postTitle. Going with window.dt.postTitle for now as it's more explicit and doesn't require an eslint exception, but please let me know if you have a better suggestion.


// If we are on a non-supported post status, change what we show
if (
dtGutenberg.supportedPostStati &&
! dtGutenberg.supportedPostStati.includes( postStatus )
) {
distributorTopMenu?.classList.add( 'hide' );
return (
<PluginDocumentSettingPanel
title={ __( 'Distributor', 'distributor' ) }
Expand All @@ -196,6 +208,7 @@ const DistributorPlugin = () => {
);
}

distributorTopMenu?.classList.remove( 'hide' );
return (
<PluginDocumentSettingPanel
title={ __( 'Distributor', 'distributor' ) }
Expand Down
11 changes: 9 additions & 2 deletions assets/js/push.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import jQuery from 'jquery';
import _ from 'underscore';
import Mustache from 'mustache';

const { document, dt } = window;
const { document, dt, dtGutenberg } = window;

let selectedConnections = {},
searchString = '';
Expand Down Expand Up @@ -344,9 +344,14 @@ jQuery( window ).on( 'load', () => {
// Determine if we need to hide the admin bar
maybeHideAdminBar();

if ( distributorPushWrapper.classList.contains( 'loaded' ) ) {
// If the post title has changed, we need to reload the template.
if (
distributorPushWrapper.classList.contains( 'loaded' ) &&
dtGutenberg.postTitle === dtGutenberg.previousPostTitle
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
dtGutenberg.postTitle === dtGutenberg.previousPostTitle
dt.postTitle === dt.previousPostTitle

) {
return;
}
dtGutenberg.previousPostTitle = dtGutenberg.postTitle;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
dtGutenberg.previousPostTitle = dtGutenberg.postTitle;
dt.previousPostTitle = dt.postTitle;


distributorPushWrapper.classList.remove( 'message-error' );
distributorPushWrapper.classList.add( 'loaded' );
Expand Down Expand Up @@ -394,13 +399,15 @@ jQuery( window ).on( 'load', () => {
connections: mustacheData.connections,
foundConnections: mustacheData.connections.length,
showSearch: 5 < mustacheData.connections.length,
postTitle: dtGutenberg.postTitle,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
postTitle: dtGutenberg.postTitle,
postTitle: dt.postTitle,

}
);

setDisabledConnections();
} else {
distributorPushWrapper.innerHTML = template( {
connections: dtConnections,
postTitle: dtGutenberg.postTitle,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
postTitle: dtGutenberg.postTitle,
postTitle: dt.postTitle,

} );
}

Expand Down
11 changes: 4 additions & 7 deletions includes/push-ui.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function syndicatable() {

global $pagenow;

if ( 'post.php' !== $pagenow ) {
if ( 'post.php' !== $pagenow && 'post-new.php' !== $pagenow ) {
return false;
}
} else {
Expand All @@ -77,17 +77,14 @@ function syndicatable() {
}
}

global $post;
$post = get_post();

if ( empty( $post ) ) {
return;
}

if ( ! in_array( $post->post_status, \Distributor\Utils\distributable_post_statuses(), true ) ) {
return false;
}

peterwilsoncc marked this conversation as resolved.
Show resolved Hide resolved
if ( ! in_array( get_post_type(), $distributable_post_types, true ) || ( ! empty( $_GET['post_type'] ) && 'dt_ext_connection' === $_GET['post_type'] ) ) { // @codingStandardsIgnoreLine Nonce not required
$distributable_post_types = array_diff( $distributable_post_types, array( 'dt_ext_connection' ) );
if ( ! in_array( get_post_type(), $distributable_post_types, true ) ) {
return false;
}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
},
"scripts": {
"build": "wp-scripts build",
"watch": "wp-scripts build --watch",
Copy link
Collaborator

Choose a reason for hiding this comment

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

The start script on line 54 already does this (wp-scripts start being an alias).

Did you add this because watch is a more common name than start? I've been considering the same because it's a more common name.

Copy link
Contributor Author

@ggutenberg ggutenberg Mar 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I didn't realize start does the same thing. I did a search in package.json for the word watch and couldn't find anything, so I added it.

Do you want me to remove this or alias it to npm run start?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want me to remove this or alias it to npm run start?

Let's remove it for now. I'll review our other open-source plugins for the common names, whether it be dev, watch or something else.

"build:docs": "rm -rf docs-built && jsdoc -c hookdoc-conf.json distributor.php includes",
"check-engines": "wp-scripts check-engines",
"check-licenses": "wp-scripts check-licenses",
Expand Down
2 changes: 1 addition & 1 deletion templates/show-connections-amp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<div class="inner">
{{#foundConnections}}
<?php /* translators: %s the post title */ ?>
<p><?php echo sprintf( esc_html__( 'Distribute &quot;%s&quot; to other connections.', 'distributor' ), esc_html( get_the_title( $post->ID ) ) ); ?></p>
<p><?php echo sprintf( esc_html__( 'Distribute &quot;%s&quot; to other connections.', 'distributor' ), '{{ postTitle }}' ); ?></p>

<div class="connections-selector">
<div>
Expand Down
2 changes: 1 addition & 1 deletion templates/show-connections.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<div class="inner">
<# if ( ! _.isEmpty( connections ) ) { #>
<?php /* translators: %s the post title */ ?>
<p><?php echo sprintf( esc_html__( 'Distribute &quot;%s&quot; to other connections.', 'distributor' ), esc_html( get_the_title( $post->ID ) ) ); ?></p>
<p><?php echo sprintf( esc_html__( 'Distribute &quot;%s&quot; to other connections.', 'distributor' ), '{{ postTitle }}' ); ?></p>

<div class="connections-selector">
<div>
Expand Down