8000 fix: add custom site titles by rbcorrales · Pull Request #2486 · Automattic/newspack-theme · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: add custom site titles #2486

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.

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

Merged
merged 3 commits into from
May 20, 2025
Merged

fix: add custom site titles #2486

merged 3 commits into from
May 20, 2025

Conversation

rbcorrales
Copy link
Member
@rbcorrales rbcorrales commented Apr 24, 2025

All Submissions:

Changes proposed in this Pull Request:

Fixes the site title displayed for multibranded sites. It introduces a new custom site title functionality that allows filtering both the site name and URL using hooks. This is used to address an issue with handling multi-branded site title links, where the rendered URL points to the site's root instead of the brand's root.

The implementation includes:

  • A new newspack_site_title() function that supports custom site names and URLs via filters, similar to the existing newspack_the_custom_logo() function.
  • Improved template structure in site-branding.php to use this new function and avoid duplication.
  • Three new filters:
    • newspack_site_title_name: For filtering the site name that appears within the site title section.
    • newspack_site_title_url: For filtering the home URL that is linked by the site title.
    • newspack_site_title: For filtering the whole newspack_site_title() function at the end.

How 8000 to test the changes in this Pull Request:

  1. Set up a multibranded site configuration:
  • Go to Appearance > Customize > Site Identity.
  • Enable "Display Site Title".
  • Verify the site title appears correctly in the header.
  1. Test the custom filters:
  • Add a filter for newspack_site_title_name to modify the site name.
  • Add a filter for newspack_site_title_url to modify the site URL.
  • Add a filter for newspack_site_title to modify the entire site title HTML.
  • Verify each filter works as expected.
  1. Test the site title display in different contexts:
  • Check the site title on the homepage.
  • Check the site title on the post pages.
  • Verify the site title is escaped correctly and sanitized.
  • Test with special characters in the site title to ensure proper escaping.
  1. Test along with the newspack-multibranded-site plugin:
  • Download and activate the plugin with the corresponding changes from fix: add customization class to fix site title url newspack-multibranded-site#73.
  • Create and configure a brand.
  • Add pages to the brand.
  • Upon visiting them, the site title URL in the header of the branded pages should point to the root of the site brand (e.g., localhost/my-brand instead of just localhost/), just like the header logo link.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@rbcorrales rbcorrales requested a review from a team as a code owner April 24, 2025 19:06
@rbcorrales rbcorrales added the [Status] Needs Review The issue or pull request needs to be reviewed label Apr 24, 2025
Copy link
Member
@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

This is a clean solution to make it extensible and works well! Left a few comments inline.

@rbcorrales rbcorrales force-pushed the fix/custom-site-title branch from 426760a to 20f61ce Compare May 7, 2025 21:08
@rbcorrales rbcorrales requested a review from miguelpeixe May 7, 2025 21:35
Copy link
Member
@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

Thank you for the changes!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels May 12, 2025
@rbcorrales rbcorrales merged commit fc91ed9 into trunk May 20, 2025
6 checks passed
@rbcorrales rbcorrales deleted the fix/custom-site-title branch May 20, 2025 21:43
Copy link

Hey @rbcorrales, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request May 23, 2025
## [2.9.1-alpha.1](v2.9.0...v2.9.1-alpha.1) (2025-05-23)

### Bug Fixes

* add custom site titles ([#2486](#2486)) ([fc91ed9](fc91ed9))
* add filter to show the update date on post types ([#2484](#2484)) ([ec9c1bf](ec9c1bf))
* **category-links:** adjust category links to handle links in sponsor descriptions ([#2488](#2488)) ([882c271](882c271))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.9.1-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jun 2, 2025
## [2.9.1](v2.9.0...v2.9.1) (2025-06-02)

### Bug Fixes

* add custom site titles ([#2486](#2486)) ([fc91ed9](fc91ed9))
* add filter to show the update date on post types ([#2484](#2484)) ([ec9c1bf](ec9c1bf))
* **category-links:** adjust category links to handle links in sponsor descriptions ([#2488](#2488)) ([882c271](882c271))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.9.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0