From 58a556430c982e3b7db12d8dfb59f55cc4609a18 Mon Sep 17 00:00:00 2001 From: Suguru Inoue Date: Mon, 16 Jun 2025 14:50:57 +0200 Subject: [PATCH] fix(editor): Fix log view style bugs (#16312) --- .../frontend/editor-ui/src/__tests__/setup.ts | 2 +- .../logs/components/LogsOverviewRow.vue | 20 ++++--- .../logs/composables/usePiPWindow.test.ts | 6 ++ .../features/logs/composables/usePiPWindow.ts | 57 +++++++++++++++++++ .../frontend/editor-ui/src/stores/ui.store.ts | 38 ++++++------- .../frontend/editor-ui/src/stores/ui.utils.ts | 38 ++++--------- 6 files changed, 105 insertions(+), 56 deletions(-) diff --git a/packages/frontend/editor-ui/src/__tests__/setup.ts b/packages/frontend/editor-ui/src/__tests__/setup.ts index 35e10792c7..5aae98112d 100644 --- a/packages/frontend/editor-ui/src/__tests__/setup.ts +++ b/packages/frontend/editor-ui/src/__tests__/setup.ts @@ -55,7 +55,7 @@ global.IntersectionObserver = IntersectionObserver; // Mocks for useDeviceSupport Object.defineProperty(window, 'matchMedia', { writable: true, - value: vi.fn().mockImplementation((query) => ({ + value: vi.fn((query) => ({ matches: true, media: query, onchange: null, diff --git a/packages/frontend/editor-ui/src/features/logs/components/LogsOverviewRow.vue b/packages/frontend/editor-ui/src/features/logs/components/LogsOverviewRow.vue index 157e61c125..2485e5ed23 100644 --- a/packages/frontend/editor-ui/src/features/logs/components/LogsOverviewRow.vue +++ b/packages/frontend/editor-ui/src/features/logs/components/LogsOverviewRow.vue @@ -190,8 +190,9 @@ watch( - - + /> @@ -242,6 +243,7 @@ watch( overflow: hidden; position: relative; z-index: 1; + padding-inline-end: var(--spacing-5xs); & > * { overflow: hidden; @@ -269,7 +271,7 @@ watch( } .selected:not(:hover).error & { - background-color: var(--color-danger-tint-2); + background-color: var(--color-callout-danger-background); } } @@ -342,6 +344,10 @@ watch( .compactErrorIcon { flex-grow: 0; flex-shrink: 0; + width: 26px; + display: flex; + align-items: center; + justify-content: center; .container:hover & { display: none; @@ -377,10 +383,6 @@ watch( align-items: center; justify-content: center; - &:last-child { - margin-inline-end: var(--spacing-5xs); - } - &:hover { background: transparent; } diff --git a/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.test.ts b/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.test.ts index e90edbfb32..ebc2aaf8f1 100644 --- a/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.test.ts +++ b/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.test.ts @@ -3,6 +3,8 @@ import { computed, defineComponent, h, ref } from 'vue'; import { usePiPWindow } from './usePiPWindow'; import { waitFor } from '@testing-library/vue'; import { renderComponent } from '@/__tests__/render'; +import { setActivePinia } from 'pinia'; +import { createTestingPinia } from '@pinia/testing'; describe(usePiPWindow, () => { const documentPictureInPicture: NonNullable = { @@ -15,6 +17,10 @@ describe(usePiPWindow, () => { }) as unknown as Window, }; + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })); + }); + describe('canPopOut', () => { it('should return false if window.documentPictureInPicture is not available', () => { const MyComponent = defineComponent({ diff --git a/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.ts b/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.ts index 45e38dee16..cae30d273e 100644 --- a/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.ts +++ b/packages/frontend/editor-ui/src/features/logs/composables/usePiPWindow.ts @@ -1,9 +1,12 @@ import { IsInPiPWindowSymbol } from '@/constants'; +import { useUIStore } from '@/stores/ui.store'; +import { applyThemeToBody } from '@/stores/ui.utils'; import { useProvideTooltipAppendTo } from '@n8n/design-system/composables/useTooltipAppendTo'; import { computed, type ComputedRef, onBeforeUnmount, + onScopeDispose, provide, type Ref, ref, @@ -26,6 +29,35 @@ interface UsePiPWindowReturn { pipWindow?: Ref; } +function isStyle(node: Node): node is HTMLElement { + return ( + node instanceof HTMLStyleElement || + (node instanceof HTMLLinkElement && node.rel === 'stylesheet') + ); +} + +function syncStyleMutations(destination: Window, mutations: MutationRecord[]) { + const currentStyles = destination.document.head.querySelectorAll('style, link[rel="stylesheet"]'); + + for (const mutation of mutations) { + for (const node of mutation.addedNodes) { + if (isStyle(node)) { + destination.document.head.appendChild(node.cloneNode(true)); + } + } + + for (const node of mutation.removedNodes) { + if (isStyle(node)) { + for (const found of currentStyles) { + if (found.isEqualNode(node)) { + found.remove(); + } + } + } + } + } +} + /** * A composable that allows to pop out given content in document PiP (picture-in-picture) window */ @@ -48,6 +80,15 @@ export function usePiPWindow({ const tooltipContainer = computed(() => isPoppedOut.value ? (content.value ?? undefined) : undefined, ); + const uiStore = useUIStore(); + const observer = new MutationObserver((mutations) => { + if (pipWindow.value) { + syncStyleMutations(pipWindow.value, mutations); + } + }); + + // Copy over dynamic styles to PiP window to support lazily imported modules + observer.observe(document.head, { childList: true, subtree: true }); provide(IsInPiPWindowSymbol, isPoppedOut); useProvideTooltipAppendTo(tooltipContainer); @@ -104,6 +145,22 @@ export function usePiPWindow({ immediate: true, }); + // It seems "prefers-color-scheme: dark" media query matches in PiP window by default + // So we're enforcing currently applied theme in the main window by setting data-theme in PiP's body element + watch( + [() => uiStore.appliedTheme, pipWindow], + ([theme, pip]) => { + if (pip) { + applyThemeToBody(theme, pip); + } + }, + { immediate: true }, + ); + + onScopeDispose(() => { + observer.disconnect(); + }); + onBeforeUnmount(() => { isUnmounting.value = true; pipWindow.value?.close(); diff --git a/packages/frontend/editor-ui/src/stores/ui.store.ts b/packages/frontend/editor-ui/src/stores/ui.store.ts index ff516c663f..5a22779e5c 100644 --- a/packages/frontend/editor-ui/src/stores/ui.store.ts +++ b/packages/frontend/editor-ui/src/stores/ui.store.ts @@ -41,6 +41,7 @@ import { FROM_AI_PARAMETERS_MODAL_KEY, IMPORT_WORKFLOW_URL_MODAL_KEY, WORKFLOW_EXTRACTION_NAME_MODAL_KEY, + LOCAL_STORAGE_THEME, } from '@/constants'; import { STORES } from '@n8n/stores'; import type { @@ -59,26 +60,21 @@ import { useWorkflowsStore } from '@/stores/workflows.store'; import { useSettingsStore } from '@/stores/settings.store'; import { dismissBannerPermanently } from '@n8n/rest-api-client'; import type { BannerName } from '@n8n/api-types'; -import { - addThemeToBody, - getPreferredTheme, - getThemeOverride, - isValidTheme, - updateTheme, -} from './ui.utils'; +import { applyThemeToBody, getThemeOverride, isValidTheme } from './ui.utils'; import { computed, ref } from 'vue'; import type { Connection } from '@vue-flow/core'; -import { useLocalStorage } from '@vueuse/core'; +import { useLocalStorage, useMediaQuery } from '@vueuse/core'; import type { EventBus } from '@n8n/utils/event-bus'; import type { ProjectSharingData } from '@/types/projects.types'; +import identity from 'lodash/identity'; let savedTheme: ThemeOption = 'system'; try { const value = getThemeOverride(); - if (isValidTheme(value)) { + if (value !== null) { savedTheme = value; - addThemeToBody(value); + applyThemeToBody(value); } } catch (e) {} @@ -87,7 +83,13 @@ type UiStore = ReturnType; export const useUIStore = defineStore(STORES.UI, () => { const activeActions = ref([]); const activeCredentialType = ref(null); - const theme = ref(savedTheme); + const theme = useLocalStorage(LOCAL_STORAGE_THEME, savedTheme, { + writeDefaults: false, + serializer: { + read: (value) => (isValidTheme(value) ? value : savedTheme), + write: identity, + }, + }); const modalsById = ref>({ ...Object.fromEntries( [ @@ -232,12 +234,10 @@ export const useUIStore = defineStore(STORES.UI, () => { const workflowsStore = useWorkflowsStore(); const rootStore = useRootStore(); - // Keep track of the preferred theme and update it when the system preference changes - const preferredTheme = getPreferredTheme(); - const preferredSystemTheme = ref(preferredTheme.theme); - preferredTheme.mediaQuery?.addEventListener('change', () => { - preferredSystemTheme.value = getPreferredTheme().theme; - }); + const isDarkThemePreferred = useMediaQuery('(prefers-color-scheme: dark)'); + const preferredSystemTheme = computed(() => + isDarkThemePreferred.value ? 'dark' : 'light', + ); const appliedTheme = computed(() => { return theme.value === 'system' ? preferredSystemTheme.value : theme.value; @@ -352,7 +352,7 @@ export const useUIStore = defineStore(STORES.UI, () => { const setTheme = (newTheme: ThemeOption): void => { theme.value = newTheme; - updateTheme(newTheme); + applyThemeToBody(newTheme); }; const setMode = (name: keyof Modals, mode: string): void => { @@ -550,7 +550,7 @@ export const useUIStore = defineStore(STORES.UI, () => { sidebarMenuCollapsed, sidebarMenuCollapsedPreference, bannerStack, - theme, + theme: computed(() => theme.value), modalsById, currentView, isAnyModalOpen, diff --git a/packages/frontend/editor-ui/src/stores/ui.utils.ts b/packages/frontend/editor-ui/src/stores/ui.utils.ts index 19e147f494..45edd6e090 100644 --- a/packages/frontend/editor-ui/src/stores/ui.utils.ts +++ b/packages/frontend/editor-ui/src/stores/ui.utils.ts @@ -1,11 +1,12 @@ -import type { AppliedThemeOption, ThemeOption } from '@/Interface'; -import { useStorage } from '@/composables/useStorage'; import { LOCAL_STORAGE_THEME } from '@/constants'; +import type { AppliedThemeOption, ThemeOption } from '@/Interface'; -const themeRef = useStorage(LOCAL_STORAGE_THEME); - -export function addThemeToBody(theme: AppliedThemeOption) { - window.document.body.setAttribute('data-theme', theme); +export function applyThemeToBody(theme: ThemeOption, window_?: Window) { + if (theme === 'system') { + (window_ ?? window).document.body.removeAttribute('data-theme'); + } else { + (window_ ?? window).document.body.setAttribute?.('data-theme', theme); // setAttribute can be missing in jsdom environment + } } export function isValidTheme(theme: string | null): theme is AppliedThemeOption { @@ -13,29 +14,12 @@ export function isValidTheme(theme: string | null): theme is AppliedThemeOption } // query param allows overriding theme for demo view in preview iframe without flickering -export function getThemeOverride() { - return getQueryParam('theme') || themeRef.value; +export function getThemeOverride(): AppliedThemeOption | null { + const override = getQueryParam('theme') ?? localStorage.getItem(LOCAL_STORAGE_THEME); + + return isValidTheme(override) ? override : null; } function getQueryParam(paramName: string): string | null { return new URLSearchParams(window.location.search).get(paramName); } - -export function updateTheme(theme: ThemeOption) { - if (theme === 'system') { - window.document.body.removeAttribute('data-theme'); - themeRef.value = null; - } else { - addThemeToBody(theme); - themeRef.value = theme; - } -} - -export function getPreferredTheme(): { theme: AppliedThemeOption; mediaQuery: MediaQueryList } { - const isDarkModeQuery = !!window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)'); - - return { - theme: isDarkModeQuery?.matches ? 'dark' : 'light', - mediaQuery: isDarkModeQuery, - }; -}