-
Notifications
You must be signed in to change notification settings - Fork 2
CMS-45730 Add alloy template #139
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
base: main
Are you sure you want to change the base?
Conversation
892c021 to
0277cd3
Compare
0277cd3 to
1458a0a
Compare
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.
Pull Request Overview
This PR adds a new alloy-template to the templates directory, providing a Next.js-based starter template for Optimizely CMS projects. The template includes pre-configured components, TypeScript configuration, and Tailwind CSS styling.
- Adds a complete Next.js 15.5.4 template with React 19.1.0
- Includes various reusable CMS components (Teaser, Notice, Contact, etc.)
- Configures TypeScript, Tailwind CSS v4, and PostCSS
- Updates workspace configuration to include the templates directory
Reviewed Changes
Copilot reviewed 28 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/alloy-template/package.json | Defines project dependencies including Next.js, React 19, and Tailwind CSS v4 |
| templates/alloy-template/tsconfig.json | TypeScript configuration with Next.js and bundler module resolution |
| templates/alloy-template/src/components/*.tsx | Component files for Teaser, Notice, Contact, Editorial, Article, and other CMS content types |
| templates/alloy-template/src/components/Start.tsx | Start page component with composition support |
| templates/alloy-template/src/app/*.tsx | Next.js app router pages including layout, dynamic routes, and preview |
| templates/alloy-template/next.config.ts | Next.js configuration for remote image patterns |
| pnpm-workspace.yaml | Updated to include templates directory in workspace |
| pnpm-lock.yaml | Lock file updated with new template dependencies |
Files not reviewed (2)
- pnpm-lock.yaml: Language not supported
- templates/alloy-template/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type: 'string', | ||
| displayName: 'Page Description', | ||
| }, | ||
| disable_indexing: { |
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.
SEO properties should probably be grouped into a block/component. Then use the block as a property on page types. Might make more sense to define them in a contract when we have that support, but until then a block property makes sense.
We have both title and site_title. Same with description.
| displayName: 'Button Link', | ||
| group: 'Information', | ||
| }, | ||
| button_text: { |
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.
Is this button really needed? If yes, then define it as a block/compontent and use it as a block property rather than two separate properties here.
…out and content handling; enhance Teaser component with link wrapping functionality.
…mple and update page.tsx to use the new variable
…Header and update layout structure
1458a0a to
baf3440
Compare
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.
Pull Request Overview
Copilot reviewed 28 out of 36 changed files in this pull request and generated 5 comments.
Files not reviewed (2)
- pnpm-lock.yaml: Language not supported
- templates/alloy-template/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,52 @@ | |||
| import { contentType } from '@optimizely/cms-sdk'; | |||
|
|
|||
| const PageList = contentType({ | |||
Copilot
AI
Nov 11, 2025
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 PageList component only defines the content type but doesn't export a React component for rendering. Either export the content type definition or add a corresponding React component that renders the page list.
| interface HeaderProps { | ||
| currentPath: any; |
Copilot
AI
Nov 11, 2025
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 currentPath prop uses any type which defeats TypeScript's type safety. Define a proper interface for the currentPath structure, e.g., currentPath: { children?: any[]; ancestors?: Array<{ _metadata: { displayName: string; url: { hierarchical: string } } }> }.
| <div className="md:w-1/2 h-64 md:h-auto overflow-hidden"> | ||
| <img | ||
| src={opti.image?.url.default} | ||
| alt="teaser_image" |
Copilot
AI
Nov 11, 2025
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 alt text 'teaser_image' is not descriptive. Use the heading or text content as alt text for better accessibility, e.g., alt={opti.heading || 'Teaser image'}.
| {/* Heading and Description */} | ||
| <div className="space-y-4"> | ||
| <h1 | ||
| {...pa('title')} |
Copilot
AI
Nov 11, 2025
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 preview attribute is set to 'title' but the actual property being displayed is opti.heading. This should be {...pa('heading')} to match the property name defined in the content type.
| {...pa('title')} | |
| {...pa('heading')} |
| <OptimizelyExperience | ||
| nodes={opti.composition.nodes ?? []} | ||
| ComponentWrapper={ComponentWrapper} | ||
| /> | ||
| </div> |
Copilot
AI
Nov 11, 2025
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 OptimizelyExperience component is placed inside the sidebar div but appears after the closing tag of the sidebar. It should be moved outside the grid container or properly nested within the layout structure.
| <OptimizelyExperience | |
| nodes={opti.composition.nodes ?? []} | |
| ComponentWrapper={ComponentWrapper} | |
| /> | |
| </div> | |
| </div> | |
| <OptimizelyExperience | |
| nodes={opti.composition.nodes ?? []} | |
| ComponentWrapper={ComponentWrapper} | |
| /> |
This template is 75% complete. The remaining features can be implemented once the tasks in CMS-46339 are resolved.