-
Notifications
You must be signed in to change notification settings - Fork 193
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: Add campaign title block #7648
base: epic/campaigns
Are you sure you want to change the base?
Conversation
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.
@pauloiankoski I'm trying to compile it with npm run dev
but I'm receiving multiple errors in the console as you can see here:
ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\CampaignTitleBlock\edit.tsx
23:35-42
[tsl] ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\CampaignTitleBlock\edit.tsx(23,36)
TS2339: Property 'getSite' does not exist on type 'never'.
ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\shared\hooks\useCampaignId.ts
5:47-65
[tsl] ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\shared\hooks\useCampaignId.ts(5,48)
TS2339: Property 'getCurrentPostType' does not exist on type 'never'.
ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\shared\hooks\useCampaignId.ts
8:41-63
[tsl] ERROR in C:\Users\glaub\Local Sites\givewp\app\public\wp-content\plugins\give\src\Campaigns\Blocks\shared\hooks\useCampaignId.ts(8,42)
TS2339: Property 'getEditedPostAttribute' does not exist on type 'never'.
Any idea?
P.S.: I already removed the node_modules
folder and ran the npm install
again just to make sure.
@glaubersilva We might need to add |
@pauloiankoski Thanks for the update! I was able to compile it and see the block working on the campaign page. However, when I created a new regular page and added the block, I wasn't able to select a campaign because the selector only displayed the "No campaigns found" message. One more thing I would like to clarify about the page title of the campaign page (CPT). Is the expected behavior to display the string "No title" and prevent users from editing it? |
The issue with the title is something to be addressed in a separated task where we may be syncing the Campaign title with the landing page title. |
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.
@pauloiankoski I did the first round of reviews and left a few comments with some suggestions. Tomorrow I'll do another round.
{hasResolved && campaign && ( | ||
<InspectorControls> | ||
<PanelBody title="Settings" initialOpen={true}> | ||
<BaseControl label="Title"> |
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.
The BaseControl
component requires an id
attribute.
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.
https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/base-control/README.md#id
Per the docs, the only required attribute is children
. Anyway, added here: 817d364
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.
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.
@pauloiankoski Nice work! I left just a few more comments in my second round of reviews. As a note, I was a bit picky with the details while reviewing the code, as this is the first block that will serve as the foundation for others so I was trying to make sure we'll have a solid base for the next blocks. Thanks in advance for your understanding. 🙂
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.
@pauloiankoski Thanks for the changes, great work man! 🚀
Resolves GIVE-1959
Description
This pull request lays the foundation for future Campaign-related blocks by introducing key structural elements. The updates in this PR include:
campaignId
as a field to the REST API response for the Campaign Page CPT.These changes establish a scalable and flexible structure for Campaign blocks, enabling more streamlined development of future features.
Visuals
Testing Instructions
Pre-review Checklist
@unreleased
tags included in DocBlocks