Skip to content

Add post title link in template editor#66305

Open
imrraaj wants to merge 1 commit into
WordPress:trunkfrom
imrraaj:fix/post-title-link
Open

Add post title link in template editor#66305
imrraaj wants to merge 1 commit into
WordPress:trunkfrom
imrraaj:fix/post-title-link

Conversation

@imrraaj

@imrraaj imrraaj commented Oct 22, 2024

Copy link
Copy Markdown
Contributor

What?

This PR adds the link tag to post title when selected in the template editor.
For better understanding go through the issue

Why?

Fixes: #65691

How?

This PR adds the necessary anchor tag when we have link option enabled.

Testing Instructions

  • Head over to the template editor and add the Title block to it.
  • Enable the "Make title a link" block setting in the editor sidebar.
  • Review the block markup in the page source. There will be a anchor tag.

Screenshots or screencast

Before

Screenshot 2024-10-22 at 1 35 35 PM

After

Screenshot 2024-10-22 at 1 35 51 PM

@imrraaj imrraaj marked this pull request as ready for review October 22, 2024 12:20
@imrraaj imrraaj requested a review from ajitbohra as a code owner October 22, 2024 12:20
@github-actions

github-actions Bot commented Oct 22, 2024

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: imrraaj <imrraaj@git.wordpress.org>
Co-authored-by: akasunil <sunil25393@git.wordpress.org>
Co-authored-by: ivan-ottinger <ivanottinger@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@akasunil akasunil added [Type] Bug An existing feature does not function as intended [Block] Post Title Affects the Post Title Block labels Oct 22, 2024
@akasunil

Copy link
Copy Markdown
Member

Thanks for your contribution, @imrraaj.

PR works fine. We could use little bit of code refactor here, there are too many conditions, We can optimize it perhaps to make the code more readable.

@imrraaj

imrraaj commented Oct 23, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the review @akasunil. I am not sure about the correct approach as to how to refactor the code, It seems all the conditions are necessary and we can not merge them easily. Can you share your thoughts?

/* If the postType or postId is not set, It might be in template editor. Just show the title as a link.
* This is a fallback for the case where the postType and postId are not set.
*/
titleElement = (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with @akasunil that we should refactor in here, but at least for this PR we could just:

let titleElement = ! isLink ? (
		<TagName { ...blockProps }>{ __( 'Title' ) }</TagName>
	) : (
		<TagName { ...blockProps }>
			<a
				href="#post-title-pseudo-link"
				onClick={ ( event ) => event.preventDefault() }
			>
				{ __( 'Title' ) }
			</a>
		</TagName>
	);

when we set the default titleElement above (let titleElement = <TagName { ...blockProps }>{ __( 'Title' ) }</TagName>;). Note that we don't need all a attributes and not dangerouslySetInnerHTML.

@imrraaj it seems it's been a while for this PR, but can you update and rebase in order to land?

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

Labels

[Block] Post Title Affects the Post Title Block [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Title block: When the "Make title a link" setting in the Template editor is enabled, the markup of the block is missing anchor tag

3 participants