feat(cms): add or align schema for CTA component#204
Conversation
- Remove deprecated widthPercent from CTA component - Add variant enum (primary, secondary) - Make heading and body optional - Add sections.cta to container-slot content dynamic zone Resolves #148 Made-with: Cursor
Made-with: Cursor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughMakes CTA fields optional (heading, body, buttonLink), removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/cms/src/components/sections/cta.json (1)
25-29: Consider a default CTA variant to avoid null-style branches.Since
variantis optional, a default can keep rendering behavior consistent when editors do not choose one.Suggested tweak
"variant": { "type": "enumeration", "enum": ["primary", "secondary"], - "required": false + "required": false, + "default": "primary" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/components/sections/cta.json` around lines 25 - 29, The schema's optional "variant" field can result in inconsistent rendering; set a default to ensure fallback behavior. Update the "variant" property in cta.json (the "variant" enumeration) to include a "default" (e.g., "primary") so editors who omit the field still get a deterministic CTA variant at runtime.apps/cms/src/components/sections/container-slot.json (1)
6-6: Update the description example list to include CTA.Line [6] gives examples of dynamic content types, but Line [23] now enables CTA. Consider adding CTA to the description to keep editor-facing metadata accurate.
Suggested tweak
- "description": "Single slot with grid span and dynamic content (e.g. media collection, text, related questions, Bible quotes carousel)" + "description": "Single slot with grid span and dynamic content (e.g. media collection, text, related questions, CTA, Bible quotes carousel)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/components/sections/container-slot.json` at line 6, Update the "description" value in container-slot.json to include "CTA" in the example list so editor-facing metadata matches the enabled CTA functionality; locate the "description" property in the container-slot.json content (the string starting with "Single slot with grid span and dynamic content (e.g. ...") and append or insert "CTA" into the parenthetical examples alongside media collection, text, related questions, and Bible quotes carousel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/cms/src/components/sections/container-slot.json`:
- Line 6: Update the "description" value in container-slot.json to include "CTA"
in the example list so editor-facing metadata matches the enabled CTA
functionality; locate the "description" property in the container-slot.json
content (the string starting with "Single slot with grid span and dynamic
content (e.g. ...") and append or insert "CTA" into the parenthetical examples
alongside media collection, text, related questions, and Bible quotes carousel.
In `@apps/cms/src/components/sections/cta.json`:
- Around line 25-29: The schema's optional "variant" field can result in
inconsistent rendering; set a default to ensure fallback behavior. Update the
"variant" property in cta.json (the "variant" enumeration) to include a
"default" (e.g., "primary") so editors who omit the field still get a
deterministic CTA variant at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd85adaa-14b0-42d8-8a1c-478c0af044d8
📒 Files selected for processing (4)
apps/cms/schema.graphqlapps/cms/src/components/sections/container-slot.jsonapps/cms/src/components/sections/cta.jsonpackages/graphql/src/graphql-env.d.ts
- container-slot: include both sections.cta and sections.bible-quotes-carousel - schema.graphql / graphql-env.d.ts: ContainerSlotContentDynamicZone union includes both ComponentSectionsCta and ComponentSectionsBibleQuotesCarousel Made-with: Cursor
Allow the CTA component body to accept rich text (RTF) so editors can use formatted content (bold, links, etc.) in the Strapi admin instead of plain text only. Made-with: Cursor
When buttonLink is empty, the CTA button acts as an accordion trigger: the body (richtext) holds the expandable content. When buttonLink is set, the CTA is a traditional button and body is typically left empty. Regenerated schema.graphql so buttonLink is nullable in GraphQL. Made-with: Cursor
Follow-up commits (why they were made)1. feat(cms): cta body as richtext for rich content
2. feat(cms): make cta buttonLink optional for accordion-style usage
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cms/src/components/sections/cta.json (1)
26-29: Set a default forvariantto keep styling deterministic.
variantis optional with no default, so new CTA entries can remainnull, forcing downstream fallback logic. Adding"default": "primary"will stabilize rendering for future entries. Note that existing records withnullvalues will not be auto-backfilled and may require a manual migration.Proposed schema tweak
"variant": { "type": "enumeration", "enum": ["primary", "secondary"], + "default": "primary", "required": false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/components/sections/cta.json` around lines 26 - 29, The "variant" field in the CTA schema is optional and has no default, causing null/unstable styling; update the "variant" property in apps/cms/src/components/sections/cta.json (the "variant" enumeration block) to include a default value of "primary" (e.g., add "default": "primary") while keeping "required": false and the existing enum ["primary","secondary"] so new records default to "primary"; note this does not backfill existing null records which will need a separate migration if required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cms/src/components/sections/cta.json`:
- Around line 12-16: The web consumer
(apps/web/src/components/sections/CTASection.tsx) still treats body as a plain
string and doesn't query or use variant, so update the GraphQL fragment used by
CTASection.tsx to include the variant field and ensure the fragment’s body field
is fetched as the richtext shape; in the component add null guards around
heading and body (avoid rendering empty <h2> when heading is missing) and
replace the direct <p>{body}</p> rendering with the project’s richtext renderer
(e.g., renderRichText or the app’s canonical richtext component) to correctly
render richtext objects; also consume or handle the variant field in
CTASection.tsx (use it to pick styles/markup or document it as unused) so the
component and schema stay aligned.
---
Nitpick comments:
In `@apps/cms/src/components/sections/cta.json`:
- Around line 26-29: The "variant" field in the CTA schema is optional and has
no default, causing null/unstable styling; update the "variant" property in
apps/cms/src/components/sections/cta.json (the "variant" enumeration block) to
include a default value of "primary" (e.g., add "default": "primary") while
keeping "required": false and the existing enum ["primary","secondary"] so new
records default to "primary"; note this does not backfill existing null records
which will need a separate migration if required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82fa145c-205b-4523-8ed2-1403c2678831
📒 Files selected for processing (2)
apps/cms/schema.graphqlapps/cms/src/components/sections/cta.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cms/schema.graphql
- CTA variant: add default "primary" so new entries get deterministic styling when editors omit the field (CodeRabbit nitpick). - Container-slot: add CTA to description example list so editor-facing metadata matches enabled components (CodeRabbit nitpick). Made-with: Cursor
|
CodeRabbit feedback – addressed within #148 bounded context In-scope (done in this PR):
Out of scope for this PR (bounded context): |
1 participant
Resolves #148
Summary
widthPercentfrom CTA component schemavariantenum (primary, secondary) for web stylingheadingandbodyoptional (optional heading/description per issue)sections.ctato container-slotcontentdynamic zone@forge/graphqlclientContracts Changed
Regeneration Required
Validation
pnpm run check:generatedpasses (after regeneration)Made with Cursor
Summary by CodeRabbit
New Features
Updates