Skip to content

Feature/vscode-theming#833

Merged
nasdan merged 6 commits intofeature/#812-implement-vscode-ext-and-mcpfrom
feature/vscode-theming
May 8, 2026
Merged

Feature/vscode-theming#833
nasdan merged 6 commits intofeature/#812-implement-vscode-ext-and-mcpfrom
feature/vscode-theming

Conversation

@Ivanruii
Copy link
Copy Markdown
Collaborator

No description provided.

@@ -0,0 +1,33 @@
import { isVSCodeEnv } from '#common/utils/env.utils.ts';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { isVSCodeEnv, onMessage } from '#common/utils';

if (type && IFRAME_READY_TYPES.has(type)) sendTheme();
};
window.addEventListener('message', onIframeReady);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MutationObserver dispara sendTheme() en cada mutación de atributo, sin debounce. Un cambio de tema en VS Code puede generar varias mutaciones seguidas enviando el payload al iframe repetidamente. Sugerencia: agrupar con requestAnimationFrame:

let rafId = 0;
const sendThemeDebounced = () => {
  cancelAnimationFrame(rafId);
  rafId = requestAnimationFrame(sendTheme);
};
const observer = new MutationObserver(sendThemeDebounced);

import { useEffect } from 'react';

const CSS_VAR_MAP: Record<keyof ThemePayload, readonly string[]> = {
background: ['--primary-100', '--primary-500', '--primary-200'],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background se mapea al mismo valor en tres CSS vars distintas: --primary-100, --primary-500, --primary-200. Normalmente representan shades distintos del color primario; igualarlos puede producir una UI sin contraste en modo VS Code. Si es un placeholder intencional, añadir un comentario que lo indique.

document.body.appendChild(iframe);

setupBridge(iframe, appOrigin);
setupThemeSync(iframe, appOrigin);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

El retorno de setupThemeSync (función de cleanup que elimina el listener y desconecta el MutationObserver) se descarta. Para un webview cuyo ciclo de vida es el de la página es tolerable, pero es preferible guardar la referencia para hacer explícita la intención:

const _cleanupTheme = setupThemeSync(iframe, appOrigin);

root.style.setProperty(cssVar, value);
}
}
if (theme.background) document.body.style.backgroundColor = theme.background;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document.body.style.backgroundColor y document.body.style.color se establecen como estilos inline. Mezclarlo con el sistema de CSS vars puede causar conflictos de especificidad. Preferir document.documentElement.style.setProperty para mantener todo en el mismo mecanismo, igual que se hace con el resto de CSS vars en el bucle de arriba.

Comment thread apps/web/src/scenes/main.scene.tsx Outdated
import classes from './main.module.css';

import { isHeadlessEnv } from '#common/utils/env.utils.ts';
import { isHeadlessEnv, isVSCodeEnv } from '#common/utils/env.utils.ts';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { isHeadlessEnv, isVSCodeEnv } from '#common/utils/env.utils.ts';
import { isHeadlessEnv, isVSCodeEnv } from '#common/utils';

@nasdan nasdan merged commit 2a44175 into feature/#812-implement-vscode-ext-and-mcp May 8, 2026
2 of 3 checks passed
@nasdan nasdan deleted the feature/vscode-theming branch May 8, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants