Refactor templates#199
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the Handlebars templates by centralizing the HTML shell into a shared partial, simplifying specialized layouts, and removing the contributor-bot feature/assets.
Changes:
- Added a reusable
layout-shellpartial and updated multiple layouts to delegate to it. - Consolidated landing-page variants into a single
body-landingpartial with conditional navigation behavior. - Removed contributor-bot markup, JS, and CSS.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/partials/main.hbs | Removed legacy main partial content (moved into body.hbs). |
| src/partials/layout-shell.hbs | New shared HTML document shell used by multiple layouts. |
| src/partials/head-rest-api.hbs | New REST API head additions (Redoc styling/metadata). |
| src/partials/contributor-bot.hbs | Removed contributor-bot partial. |
| src/partials/body.hbs | Inlined former main partial content into body. |
| src/partials/body-rest-api.hbs | New REST API body partial rendering page.contents. |
| src/partials/body-landing.hbs | Reworked landing body to be content-driven with conditional nav variants. |
| src/partials/body-landing-tutorials.hbs | Removed in favor of body-landing + nav option. |
| src/partials/body-landing-top-level-sdk.hbs | Removed in favor of body-landing + nav option. |
| src/partials/body-landing-sdk.hbs | Removed in favor of body-landing + nav option. |
| src/partials/body-landing-page-capella.hbs | Removed in favor of body-landing + capella options. |
| src/partials/body-landing-core-concept.hbs | Removed in favor of body-landing + nav option. |
| src/partials/article.hbs | Removed contributor-bot include from articles. |
| src/layouts/tutorials.hbs | Switched to layout-shell with body-tutorials. |
| src/layouts/rest-api.hbs | Switched to layout-shell with REST API head/body partials. |
| src/layouts/landing-page.hbs | Switched to layout-shell + body-landing. |
| src/layouts/landing-page-tutorials.hbs | Switched to layout-shell + body-landing with tutorials nav variant. |
| src/layouts/landing-page-top-level-sdk.hbs | Switched to layout-shell + body-landing with sidebar variant. |
| src/layouts/landing-page-sdk.hbs | Switched to layout-shell + body-landing with full nav. |
| src/layouts/landing-page-core-concept.hbs | Switched to layout-shell + body-landing with full nav. |
| src/layouts/landing-page-capella.hbs | Switched to layout-shell + body-landing with capella styling options. |
| src/layouts/home.hbs | Switched to layout-shell and siteTitleOnly title behavior. |
| src/layouts/default.hbs | Switched to layout-shell with default title. |
| src/layouts/404.hbs | Switched to layout-shell with 404 body/template settings. |
| src/js/08-contributor-bot.js | Removed contributor-bot JS. |
| src/css/site.css | Dropped import of contributor.css. |
| src/css/contributor.css | Removed contributor-bot styling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <style> | ||
| #redoc { width: 100%} | ||
| </style> | ||
| <meta name='viewport' content='width=device-width, initial-scale=1' /> |
There was a problem hiding this comment.
head-first already includes a viewport meta tag (see src/partials/head-first.hbs:3), so adding another viewport meta here will result in duplicate viewport declarations on REST API pages. Remove the viewport meta from this partial and keep it centralized in head-first (or make head-first conditional).
| <meta name='viewport' content='width=device-width, initial-scale=1' /> |
| @@ -0,0 +1,4 @@ | |||
| <style> | |||
| #redoc { width: 100%} | |||
There was a problem hiding this comment.
The #redoc inline CSS rule is missing a trailing semicolon (width: 100%). Please add the semicolon to avoid issues when additional declarations are appended/minified.
| #redoc { width: 100%} | |
| #redoc { width: 100%; } |
| {{> layout-shell | ||
| siteTitleOnly=true | ||
| bodyClass="home" | ||
| bodyTemplate="body-home"}} |
There was a problem hiding this comment.
This changes the home page <title> from a fixed "Couchbase Documentation" string to site.title via siteTitleOnly=true. If this PR is intended to be a refactor-only change, consider preserving the previous title text (or explicitly documenting this behavior change).
| <img class="opening-image" src="{{{uiRootPath}}}/img/stars.svg" /> | ||
| {{/if}} | ||
| {{{page.contents}}} | ||
| {{#if capellaImages}} | ||
| <img class="ending-image" src="{{{uiRootPath}}}/img/lineplus.svg" /> |
There was a problem hiding this comment.
The decorative Capella images are missing alt text. Please add alt="" (and optionally aria-hidden="true" / role="presentation") so screen readers don’t announce the filenames/URLs.
| <img class="opening-image" src="{{{uiRootPath}}}/img/stars.svg" /> | |
| {{/if}} | |
| {{{page.contents}}} | |
| {{#if capellaImages}} | |
| <img class="ending-image" src="{{{uiRootPath}}}/img/lineplus.svg" /> | |
| <img class="opening-image" src="{{{uiRootPath}}}/img/stars.svg" alt="" aria-hidden="true" /> | |
| {{/if}} | |
| {{{page.contents}}} | |
| {{#if capellaImages}} | |
| <img class="ending-image" src="{{{uiRootPath}}}/img/lineplus.svg" alt="" aria-hidden="true" /> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </html> | ||
| {{> layout-shell | ||
| bodyClass="landing-page" | ||
| defaultTitle="Landing Page" |
There was a problem hiding this comment.
defaultTitle is capitalized as "Landing Page" here, but other landing-page layouts in this PR use "landing page" (and the previous template used "landing page"). This will change the browser title casing only for some landing pages; consider standardizing the default title across all landing-page layouts.
| defaultTitle="Landing Page" | |
| defaultTitle="landing page" |
| color: inherit; | ||
| } | ||
|
|
||
| #redoc { width: 100%; } |
There was a problem hiding this comment.
This new rule is formatted as a single-line block, but the surrounding rules in this file use multi-line blocks (e.g., code { ... } at src/css/base.css:69-73 and #redoc code { ... } at :76-79). Consider formatting #redoc consistently to match the established style.
| #redoc { width: 100%; } | |
| #redoc { | |
| width: 100%; | |
| } |
| </html> | ||
| {{> layout-shell | ||
| bodyClass="landing-page" | ||
| defaultTitle="Landing Page" |
There was a problem hiding this comment.
defaultTitle is capitalized as "Landing Page" here, but other landing-page layouts in this PR use "landing page" (and the previous template used "landing page"). This will change the browser title casing only for some landing pages; consider standardizing the default title across all landing-page layouts.
| defaultTitle="Landing Page" | |
| defaultTitle="landing page" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <html lang="en"> | ||
| <head> | ||
| {{> head-first}} | ||
| <title>{{#if siteTitleOnly}}{{{site.title}}}{{else}}{{{detag (or page.title defaultTitle)}}}{{#with site.title}} | {{{.}}}{{/with}}{{/if}}</title> |
There was a problem hiding this comment.
In the <title> tag, site.title is rendered with triple-stash ({{{site.title}}} / {{{.}}}), which disables HTML escaping. If site.title contains &, <, or other special characters, this can produce invalid markup and potentially allow HTML injection into the document title. Prefer the escaped form ({{site.title}} / {{.}}) and let Handlebars handle entity escaping (keep detag(...) for page.title if needed).
| <title>{{#if siteTitleOnly}}{{{site.title}}}{{else}}{{{detag (or page.title defaultTitle)}}}{{#with site.title}} | {{{.}}}{{/with}}{{/if}}</title> | |
| <title>{{#if siteTitleOnly}}{{site.title}}{{else}}{{{detag (or page.title defaultTitle)}}}{{#with site.title}} | {{.}}{{/with}}{{/if}}</title> |
| <article class="doc"> | ||
| {{#if page.title}} | ||
| <div class="page-heading-title"> | ||
| <h1 class="page">{{{page.title}}}</h1> | ||
| {{> labels}} | ||
| </div> | ||
| {{/if}} | ||
| {{{page.contents}}} | ||
| {{> pagination}} | ||
| </article> |
There was a problem hiding this comment.
article.hbs previously included {{> contributor-bot}} (plus related CSS/JS). With the article markup now inlined here and the partial deleted, the contributor/last-commit UI has been removed entirely. If that feature is still expected on article pages, re-add it (either by restoring the partial or embedding it here) and ensure the supporting assets are still included; otherwise, consider updating the PR title/description to reflect this intentional functional removal (not just a refactor).
No description provided.