Skip to content

Conversation

@getdave
Copy link
Contributor

@getdave getdave commented Jun 21, 2023

Backports changes that amend the Post Edit link for wp_navigation posts from WordPress/gutenberg#39286 to Core.

Trac ticket: https://core.trac.wordpress.org/ticket/58589


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Co-authored-by: Jonny Harris <[email protected]>
Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Thanks @getdave

The code looks good.

I was just wondering, is the back link used anywhere?

Looking at WordPress/gutenberg#39286 I couldn't work out whether we need this change at all.

Or is for revisions.php?

if ( 'wp_template' === $post->post_type || 'wp_template_part' === $post->post_type ) {
$slug = urlencode( get_stylesheet() . '//' . $post->post_name );
$link = admin_url( sprintf( $post_type_object->_edit_link, $post->post_type, $slug ) );
} elseif ( 'wp_navigation' === $post->post_type ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we could add an annotation in the get_edit_post_link() PHP doc, e.g., something like

* @since 6.3.0 Adds custom link for wp_navigation post types.

@getdave
Copy link
Contributor Author

getdave commented Jun 23, 2023

@ramonjd It was on the revisions page "Back to Editor" button.

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Thanks for getting this PR up @getdave

_edit_link needs to be one level up in the register_post_type() args array

Also it'd be great to add a test

@ramonjd
Copy link
Member

ramonjd commented Jun 27, 2023

Superseded by #4710, which adds some required annotations.

Thanks @getdave !

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

LGTM

@tellthemachines
Copy link
Contributor

Closing in favour of #4710

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants