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.

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

Feature: Campaign Page template #7761

Open
wants to merge 10 commits into
base: epic/campaigns
Choose a base branch
from

Conversation

alaca
Copy link
Member

@alaca alaca commented Feb 26, 2025

Description

Registers a new campaign page template that will be used in themes that support FSE.
Also, the default Gutenberg title block is hidden and replaced with the Campaign title block.

Affects

Campaign page

Visuals

Testing Instructions

Install FSE theme
Create Campaign
Preview the campaign page on frontend
Edit site appearance

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@alaca alaca requested a review from pauloiankoski February 27, 2025 10:05
@alaca
Copy link
Member Author

alaca commented Feb 27, 2025

@pauloiankoski I added you as a reviewer

Copy link
Contributor

@pauloiankoski pauloiankoski left a comment

Choose a reason for hiding this comment

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

Left some points to discuss before approving but nothing critical has been found.

Comment on lines +34 to +48
public function loadTemplate($template)
{
if (
'give_campaign_page' === get_query_var('post_type')
&& current_theme_supports('block-templates')
) {
$template = $this->canRegisterBlockTemplate()
? $template
: GIVE_PLUGIN_DIR . '/src/Campaigns/resources/views/campaign-page-template.php';

return locate_block_template($template, 'campaign-page-template', ['campaign-page-template.php']);
}

return $template;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

When hooking into template_include, we should first check if the user’s theme provides a template before falling back to our own. In this case, single-give_campaign_page.php should be prioritized if it exists in the theme.

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole purpose of this function is to load our template for themes that support FSE because they load a ton of stuff that we don't want to see on our campaign page. That's the point of this PR. Users can always change the layout from the editor if they don't like it. For sites that don't support FSE, the dev can create single-give_campaign_page.php and it will be loaded normally.

Copy link
Contributor

Choose a reason for hiding this comment

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

CleanShot 2025-02-28 at 13 00 56

This is an example of a theme without support for FSE. I thought that campaign-page-template.php would surface exactly in cases like that. It sounds like we are implementing it partially, so we’ll need to communicate that clearly to our users.

Comment on lines +16 to +18
.edit-post-visual-editor__post-title-wrapper {
display: none;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, this should be in an editor stylesheet, but for now, it works to keep it within the CampaignTitle block since it is being loaded all the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

We use our title block only on campaign pages. This makes sense as it is loaded only on campaign pages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but the styles are not being applied to the CampaignTitle block but to the page title element instead. That's what I meant.

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.

2 participants