Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
Adjust whitespace in files related to the pro theme and slide layout builder to prepare the feature workMake no functional changes; only modify blank lines across affected files to normalize formatting. 📍Where to StartStart by scanning the diff in the main entry point for the pro theme and slide layout builder, for example index.ts, to confirm only whitespace changes are present. 📊 Macroscope summarized f089ecc. 8 files reviewed, 14 issues evaluated, 14 issues filtered, 0 comments posted. View details |
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| 'layouts', | ||
| `${payload.name}.hbs`, | ||
| ); | ||
|
|
||
| if (!(await fileExists(layoutPath))) { | ||
| ThemeBuilderPanel.postRequestMessage(command, requestId, null); | ||
| return; |
There was a problem hiding this comment.
Multiple handlers use payload.* without validation, which can throw. Suggest consistently validate payload and required fields (types) before use, and return a safe default (e.g., null or { success: false }) when invalid.
| 'layouts', | |
| `${payload.name}.hbs`, | |
| ); | |
| if (!(await fileExists(layoutPath))) { | |
| ThemeBuilderPanel.postRequestMessage(command, requestId, null); | |
| return; | |
| if (!payload || !payload.name) { | |
| ThemeBuilderPanel.postRequestMessage(command, requestId, null); | |
| return; | |
| } | |
| const layoutPath = Uri.joinPath( | |
| wsFolder.uri, | |
| General.demoFolder, | |
| 'layouts', | |
| `${payload.name}.hbs`, | |
| ); |
🚀 Want me to fix this? Reply ex: "fix it for me".
| const layoutPath = Uri.joinPath( | ||
| wsFolder.uri, | ||
| General.demoFolder, | ||
| 'layouts', | ||
| `${payload.name}.hbs`, | ||
| ); |
There was a problem hiding this comment.
loadLayout forces a .hbs suffix, so selecting a .handlebars file fails. Suggest preserve the provided extension, or try .hbs then .handlebars when resolving the file.
- const layoutPath = Uri.joinPath(
- wsFolder.uri,
- General.demoFolder,
- 'layouts',
- `${payload.name}.hbs`,
- );
+ const layoutsFolder = Uri.joinPath(
+ wsFolder.uri,
+ General.demoFolder,
+ 'layouts',
+ );
+ const fileBase = payload.name;
+ const fileName =
+ fileBase.endsWith('.hbs') || fileBase.endsWith('.handlebars')
+ ? fileBase
+ : (await fileExists(Uri.joinPath(layoutsFolder, `${fileBase}.hbs`)))
+ ? `${fileBase}.hbs`
+ : `${fileBase}.handlebars`;
+ const layoutPath = Uri.joinPath(layoutsFolder, fileName);
🚀 Want me to fix this? Reply ex: "fix it for me".
| <select | ||
| className="flex-1 px-3 py-2 border rounded" | ||
| value={selectedTheme} | ||
| onChange={(e) => loadTheme(e.target.value)} |
There was a problem hiding this comment.
Selecting the empty option triggers unnecessary loads. Suggest guard the onChange to skip calling loaders when the value is empty (or document why an empty load is intended).
🚀 Want me to fix this? Reply ex: "fix it for me".
| switch (command) { | ||
| case WebViewMessages.toVscode.themeBuilder.getExistingThemes: | ||
| await ThemeBuilderPanel.getExistingThemes(command, requestId); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.getExistingLayouts: | ||
| await ThemeBuilderPanel.getExistingLayouts(command, requestId); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.loadTheme: | ||
| await ThemeBuilderPanel.loadTheme(command, requestId, payload); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.loadLayout: | ||
| await ThemeBuilderPanel.loadLayout(command, requestId, payload); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.saveTheme: | ||
| await ThemeBuilderPanel.saveTheme(command, requestId, payload); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.saveLayout: | ||
| await ThemeBuilderPanel.saveLayout(command, requestId, payload); | ||
| break; | ||
| case WebViewMessages.toVscode.themeBuilder.getPreviewHtml: | ||
| await ThemeBuilderPanel.getPreviewHtml(command, requestId, payload); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Overriding messageListener drops the base handling for getSetting, getFileContents, and runCommand. Consider delegating unhandled commands to super.messageListener(message) so core webview commands keep working.
+ default:
+ await super.messageListener(message);
+ break;🚀 Want me to fix this? Reply ex: "fix it for me".
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
| if (result?.success) { | ||
| setSaveStatus({ type: 'success', text: 'Theme saved successfully!' }); | ||
| await loadExistingFiles(); | ||
| setTimeout(() => setSaveStatus({ type: 'blank', text: '' }), 3000); |
There was a problem hiding this comment.
setTimeout resets saveStatus after 3s, which can fire after unmount and update state on an unmounted component. Consider storing the timeout id in a useRef, clearing it before setting a new one, and clearing it in a useEffect cleanup.
🚀 Want me to fix this? Reply ex: "fix it for me".
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
| let sanitizedHtml = (payload.html || '<h1>Preview</h1><p>Add your HTML content to see the preview</p>'); | ||
|
|
||
| // Remove script tags (handle all whitespace variations including newlines and tabs) | ||
| // Using a more comprehensive pattern that matches any whitespace in closing tags |
There was a problem hiding this comment.
The script-tag sanitizer misses self-closing tags like <script/> (and similar variants), so scripts may slip through. Consider expanding the regex to also match self-closing <script .../> in addition to paired tags.
| // Using a more comprehensive pattern that matches any whitespace in closing tags | |
| sanitizedHtml = sanitizedHtml.replace(/<script\b[^>]*>([\s\S]*?)<\/\s*script\s*>|<script\b[^>]*\/\s*>/gi, '<!-- script blocked -->'); |
🚀 Want me to fix this? Reply ex: "fix it for me".
| await writeFile(layoutPath, payload.content); | ||
|
|
||
| ThemeBuilderPanel.postRequestMessage(command, requestId, { |
There was a problem hiding this comment.
sanitizeFileName can return an empty string. That would make Uri.joinPath(layoutsFolder, fileName) target the folder itself, and writeFile would fail. Consider guarding for an empty fileName and return { success: false } early.
| await writeFile(layoutPath, payload.content); | |
| ThemeBuilderPanel.postRequestMessage(command, requestId, { | |
| const fileName = sanitizeFileName(payload.name, '.hbs'); | |
| if (!fileName) { | |
| ThemeBuilderPanel.postRequestMessage(command, requestId, { success: false }); | |
| return; | |
| } | |
| const layoutPath = Uri.joinPath(layoutsFolder, fileName); |
🚀 Want me to fix this? Reply ex: "fix it for me".
| // Check again after authentication | ||
| const isNowSponsor = SponsorService.getSponsorStatus(); |
There was a problem hiding this comment.
isNowSponsor is read immediately after COMMAND.authenticate, but if auth throws, checkSponsor() never runs and the state may be stale. Consider calling SponsorService.checkSponsor() before reading getSponsorStatus() so the status is refreshed even on auth errors.
| // Check again after authentication | |
| const isNowSponsor = SponsorService.getSponsorStatus(); | |
| // Check again after authentication | |
| await SponsorService.checkSponsor(); | |
| const isNowSponsor = SponsorService.getSponsorStatus(); |
🚀 Want me to fix this? Reply ex: "fix it for me".
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
| name: file.replace('.css', ''), | ||
| path: file, | ||
| }); | ||
| } |
There was a problem hiding this comment.
In getExistingThemes, file.replace('.css', '') removes the first .css anywhere in the name, not just the extension. Consider using file.replace(/\.css$/, '') so only the file suffix is stripped.
| } | |
| name: file.replace(/\.css$/, ''), |
🚀 Want me to fix this? Reply ex: "fix it for me".
|
It needs to be an easy visual theme editor that makes it possible to create theme files ( When the user opens the theme editor, they should be able to:
Example themes: mc2mc.slide.mc2mc {
background: #fff;
color: #000000;
font-size: 24px;
font-family: "Arial", sans-serif;
.slide__layout {
background-image: url(".demo/assets/mc2mc-background.png");
background-repeat: no-repeat;
background-size: cover;
background-position: center center;
}
.slide__content__inner {
padding: calc(var(--spacing) * 8);
box-sizing: border-box;
> :not([hidden]) ~ :not([hidden]) {
margin-top: 15px;
}
}
h1 {
color: #1b9ae0;
font-size: 32px;
font-weight: 600;
}
h2 {
font-size: 30px;
}
h3 {
font-size: 28px;
}
h4 {
font-size: 26px;
}
h5 {
font-size: 24px;
}
p {
font-size: 24px;
}
a {
color: #2a62bb;
text-decoration-line: underline;
&:hover {
color: #1a4d8f;
}
}
ul,
ol {
font-size: 24px;
margin-left: 19px;
li {
margin-bottom: calc(var(--spacing) * 2);
}
ul,
ol {
margin-top: calc(var(--spacing) * 2);
}
}
ul {
list-style-type: disc;
}
ol {
list-style-type: decimal;
}
blockquote {
border-left-style: var(--tw-border-style);
border-left-width: 4px;
border-color: var(--demotime-blockquote-border);
background-color: var(--demotime-blockquote-background);
padding: calc(var(--spacing) * 2);
}
.default {
.slide__content__inner {
padding-top: 60px;
padding-left: 65px;
> :not([hidden]) ~ :not([hidden]) {
margin-top: 5px;
}
}
h1 {
margin-bottom: 25px;
}
}
.intro {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
font-size: 42px;
}
}
.quote {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
text-align: center;
}
h1 {
font-size: 38px;
}
}
.section {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
font-size: 42px;
}
h2 {
font-size: 32px;
opacity: 80%;
}
p {
opacity: 75%;
}
}
.image {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
padding: calc(var(--spacing) * 1);
}
}
.video {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
background-color: transparent;
}
.slide__video {
position: fixed;
inset: 0;
z-index: -1;
overflow: hidden;
pointer-events: none;
video {
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
min-width: 100%;
min-height: 100%;
width: auto;
height: auto;
object-fit: cover;
filter: brightness(0.65);
}
}
h1 {
padding: calc(var(--spacing) * 1);
}
}
.image-left {
.slide__content__inner {
padding-top: 60px;
}
.slide__image_left {
background-size: contain !important;
background-position-y: 65px !important;
background-position-x: center !important;
}
h1 {
margin-top: -7px;
}
}
.image-right {
.slide__content__inner {
padding-top: 60px;
}
.slide__image_right {
background-size: contain !important;
background-position-y: 65px !important;
background-position-x: center !important;
}
h1 {
margin-top: -7px;
}
}
.two-columns {
.slide__content__inner {
padding-top: 60px;
}
.slide__content__inner {
display: grid;
width: 100%;
grid-template-columns: repeat(2, minmax(0, 1fr));
gap: calc(var(--spacing) * 4);
:where(& > :not(:last-child)) {
--tw-space-y-reverse: 0;
margin-block-start: calc(calc(var(--spacing) * 0) * var(--tw-space-y-reverse));
margin-block-end: calc(calc(var(--spacing) * 0) * calc(1 - var(--tw-space-y-reverse)));
}
}
.slide__left,
.slide__right {
width: 100%;
:where(& > :not(:last-child)) {
--tw-space-y-reverse: 0;
margin-block-start: calc(calc(var(--spacing) * 4) * var(--tw-space-y-reverse));
margin-block-end: calc(calc(var(--spacing) * 4) * calc(1 - var(--tw-space-y-reverse)));
}
}
.slide__right {
margin-top: 0 !important;
}
h1 {
}
}
}ESPC25.slide.espc25 {
background: #081f37;
color: #fff;
font-size: 24px;
font-family: "Arial", sans-serif;
.slide__layout {
background-image: url(".demo/assets/background.webp");
background-repeat: no-repeat;
background-size: cover;
background-position: center center;
}
.slide__content__inner {
padding: calc(var(--spacing) * 8);
box-sizing: border-box;
> :not([hidden]) ~ :not([hidden]) {
margin-top: 15px;
}
}
h1 {
color: #00c700;
font-size: 32px;
font-weight: 600;
}
h2 {
font-size: 30px;
}
h3 {
font-size: 28px;
}
h4 {
font-size: 26px;
}
h5 {
font-size: 24px;
}
p {
font-size: 24px;
}
a {
color: #2a62bb;
text-decoration-line: underline;
&:hover {
color: #1a4d8f;
}
}
ul,
ol {
font-size: 24px;
margin-left: 19px;
li {
margin-bottom: calc(var(--spacing) * 2);
}
ul,
ol {
margin-top: calc(var(--spacing) * 2);
}
}
ul {
list-style-type: disc;
}
ol {
list-style-type: decimal;
}
blockquote {
border-left-style: var(--tw-border-style);
border-left-width: 4px;
border-color: var(--demotime-blockquote-border);
background-color: var(--demotime-blockquote-background);
padding: calc(var(--spacing) * 2);
}
.default {
.slide__content__inner {
padding-top: 60px;
padding-left: 65px;
> :not([hidden]) ~ :not([hidden]) {
margin-top: 5px;
}
}
h1 {
margin-bottom: 25px;
}
}
.intro {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
font-size: 42px;
}
}
.quote {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: flex-start;
justify-content: center;
text-align: center;
}
h1 {
font-size: 38px;
}
}
.section {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
font-size: 42px;
}
h2 {
font-size: 32px;
opacity: 80%;
}
p {
opacity: 75%;
}
}
.image {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
}
h1 {
padding: calc(var(--spacing) * 1);
}
}
.video {
.slide__content__inner {
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
text-align: center;
background-color: transparent;
}
.slide__video {
position: fixed;
inset: 0;
z-index: -1;
overflow: hidden;
pointer-events: none;
video {
position: absolute;
top: 50%;
left: 50%;
transform: translate(-50%, -50%);
min-width: 100%;
min-height: 100%;
width: auto;
height: auto;
object-fit: cover;
filter: brightness(0.65);
}
}
h1 {
padding: calc(var(--spacing) * 1);
}
}
.image-left {
.slide__content__inner {
padding-top: 60px;
}
.slide__image_left {
background-size: contain !important;
background-position-y: 65px !important;
background-position-x: center !important;
}
h1 {
margin-top: -7px;
}
}
.image-right {
.slide__content__inner {
padding-top: 60px;
}
.slide__image_right {
background-size: contain !important;
background-position-y: 65px !important;
background-position-x: center !important;
}
h1 {
margin-top: -7px;
}
}
.two-columns {
.slide__content__inner {
padding-top: 60px;
}
.slide__content__inner {
display: grid;
width: 100%;
grid-template-columns: repeat(2, minmax(0, 1fr));
gap: calc(var(--spacing) * 4);
:where(& > :not(:last-child)) {
--tw-space-y-reverse: 0;
margin-block-start: calc(
calc(var(--spacing) * 0) * var(--tw-space-y-reverse)
);
margin-block-end: calc(
calc(var(--spacing) * 0) * calc(1 - var(--tw-space-y-reverse))
);
}
}
.slide__left,
.slide__right {
width: 100%;
:where(& > :not(:last-child)) {
--tw-space-y-reverse: 0;
margin-block-start: calc(
calc(var(--spacing) * 4) * var(--tw-space-y-reverse)
);
margin-block-end: calc(
calc(var(--spacing) * 4) * calc(1 - var(--tw-space-y-reverse))
);
}
}
.slide__right {
margin-top: 0 !important;
}
h1 {
}
}
}
.slide.espc25.light {
background: #fff;
color: #000000;
} |
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
Implemented a visual theme builder with form-based UI (commit dc7d2db). Key Features: UI Structure:
Themes generate structured CSS like: .slide.themename {
background: #fff;
.intro { /* centered layout */ }
.section { /* custom styles */ }
}Files saved to |
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
| const mainContent = mainSelectorMatch[1]; | ||
|
|
||
| // Extract global properties (before any nested selectors) | ||
| const globalSection = mainContent.split(/\.[a-z_]+\s*\{/)[0]; |
There was a problem hiding this comment.
The split regex \.[a-z_]+\s*\{ misses hyphenated class names, so template blocks like image-left leak into globalSection. Consider allowing hyphens in the class part to split correctly.
| const globalSection = mainContent.split(/\.[a-z_]+\s*\{/)[0]; | |
| const globalSection = mainContent.split(/\.[a-z_-]+\s*\{/)[0]; |
🚀 Want me to fix this? Reply ex: "fix it for me".
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
Co-authored-by: estruyf <2900833+estruyf@users.noreply.github.com>
|
Implements a visual, form-based theme builder for GitHub Sponsors to create and customize slide themes without requiring CSS knowledge, addressing the difficulty of customizing styles without inspection tools.
Architecture
Backend (
ThemeBuilderService+ThemeBuilderPanel)SponsorService- prompts authentication for non-sponsors.demo/theme/*.css(themes saved to theme folder, not slides)Frontend (
ThemeBuilderView.tsx)Security: Maintained from original implementation
default-src 'none'; style-src 'unsafe-inline'Features
✅ Visual editor that guides users through theme creation (no CSS knowledge required)
✅ Create and manage themes in
.demo/theme/folder✅ Define theme name and CSS class
✅ Configure global slide styles with color pickers
✅ Add and configure slide templates
✅ Define layouts per slide template (default, center, flex-start)
✅ Add backgrounds (colors and images) globally or per slide template
✅ Save/export theme files
✅ Load existing themes with enhanced CSS parsing
✅ Live CSS preview
Example Usage
Generated CSS Structure
The visual builder generates structured CSS following this pattern:
Changes
New files:
services/ThemeBuilderService.ts- Command registration, sponsor checkpanels/ThemeBuilderPanel.ts- Webview message handling, theme file operationswebviews/ThemeBuilderView.tsx- React visual form-based editordocs/features/theme-builder.mdx- Visual editor documentationModified:
openThemeBuildercommand and messagesthemeBuilderwebview typeSecurity notes:
Original prompt
I want that you create me a theme builder that is only available as a pro feature.
The idea would be that you can create a new theme and/or slide layout.
Examples of slide layouts can be found here: https://github.com/estruyf/presentation-testing-smarter-ai-playwright/tree/main/.demo/layout
Example of themes can be found here: https://github.com/estruyf/presentation-testing-smarter-ai-playwright/tree/mc2mc/.demo/theme
Documentation for slides: https://demotime.show/slides/
The solution needs to solve: hard to figure out how to do custom styling without a way to inspect.
[Chronological Review: The conversation began with the user requesting specific modifications to slides and presentations, including avoiding certain components and formatting. The user then shifted focus to reviewing and improving various agents, particularly the `demotime-slides.agent.md` and `demotime-interview.agent.md`. The user also requested the creation of release notes and highlighted issues in the code related to state management and rendering. As the conversation progressed, the user provided detailed feedback on specific files, emphasizing the need for better documentation, analytics features, and a theme builder as a pro feature. The most recent requests involved implementing a presentation analytics suite and a theme builder.][Intent Mapping:
<dt-list>component, only when it makes sense.".demo/slides."dt-showtoo much."[Technical Inventory:
[Code Archaeology:
demotime-slides.agent.mddemotime-interview.agent.mdCHANGELOG.mdConfigEditorProvider.tsInteractionService.tsSettingField.tsxStepEditor.tsxservice.astroDemoRunner.tsZoomService.tsvscode.mdxsettings.mdx[Progress Assessment:
[Context Validation: All critical information for continuation is captured, including user requests for specific features and improvements.]
[Recent Commands Analysis:
1. Conversation Overview: - Primary Objectives: The user requested various modifications to presentation slides, improvements to agents, and the implementation of new features such as a presentation analytics suite and a theme builder. - Session Context: The conversation flowed from specific slide formatting requests to broader feature implementations aimed at enhancing user experience and customization. - User Intent Evolution: The u...
Created from VS Code.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.