-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
@valendesigns what were the todos we listed out for this PR? I added one in the description. I realize we can continue to use |
I'll update this later tonight I just got back from the airport. 😀 |
panel: panel | ||
} ) ); | ||
|
||
$( '.add-new-' + panel.postType ).on( 'click', function( event ) { |
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.
I believe this should be scoped, for example:
panel.container.find( '.add-new.button-secondary' ).on( … )
In this case, there wouldn't need to be a type-specific class name because the button would be scoped to the container.
@westonruter This PR is ready for a final review. |
public function include_previewed_drafts( $where, $query ) { | ||
global $wpdb; | ||
|
||
if ( ! $query->is_admin() && ! $query->is_singular() ) { |
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.
PhpStorm is complaining here that is_admin()
is not a method found on WP_Query
. I can't find a WP_Query::is_admin()
method either, but also it is not throwing an error for me which is also unexpected. I suppose it is because there is a magic getter defined.
So that leads me to think that $query->is_admin()
is actually always returning null
and isn't returning the bool
that you expect if accessing $query->is_admin
.
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.
is_admin()
will work.
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.
We can change it to if ( ! is_admin() && ! $query->is_singular() ) {
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.
For some reason $wp_query->is_admin()
is returning a bool. However note:
$query = new WP_Query();
$query->is_admin = true;
$this->assertTrue( $query->is_admin() );
This test fails. All of this to say, I think that $query->is_admin
should be used instead of $query->is_admin()
@westonruter Looks good! |
* Harden logic for focusing on first control in section * Reject a promise with the error message provided. * Export publicly_queryable among the post type args.
…t's URL Skip showing navigation link if post type is not publicly_queryable
…only add filter once preview()
_.defer( function() { | ||
firstControl.focus( { | ||
completeCallback: function() { | ||
firstControl.container.find( 'input:first' ).select(); |
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.
Haha. I really did do some egregious callback nesting here didn't I 😆
…ain_query_post_type filter with any post type lookup
Always return WP_Post or WP_Error, pass back to client in error message
…odules gets created to fix ESLint checks xwp/wp-dev-lib@046c3c5...4c79f77 d14fb0f Fix activation of plugins via WP_TEST_ACTIVATED_PLUGINS config 9a6eaba Merge pull request xwp/wp-dev-lib#176 from branch bugfix/test-activated-plugins e34026e Ignore `node_modules` symlinks when installing `npm`. 6242f95 Merge pull request xwp/wp-dev-lib#177 from branch bugfix/ignore-node-modules-symlink c05837c Define Paths * Define paths to the current WP install instead of using the wordpress-develop paths as per the tests bootstrap ad82ec3 Define WP_CONTENT_DIR and WP_PLUGIN_DIR 4c79f77 Ensure symlink for node_modules gets created to fix ESLint checks
Fixes #48, fixes #50
customize-draft
status which is assigned to anauto-draft
post once it has been saved as part of a snapshot. This will prevent it from being garbage-collected.