Dark mode

[markverick]

Add dark/light mode toggle with persistent theme preference

Adds a slide-switch toggle in the sidebar (NavBar) that lets users switch between light and dark themes. The preference is saved to localStorage and applied before first paint to avoid a flash of the wrong theme.

What's included

• Theme toggle switch — sun/moon slide switch in the NavBar logo row; smooth curtain overlay transition on switch
• Persistent preference — stored in localStorage('ownly.theme'); inline script in index.html applies it before Vue mounts
• Live editor updates — Monaco and Milkdown editors react to theme changes in real time via a shared useThemeWatch composable (MutationObserver on data-theme + matchMedia listener)
• System theme isolation — OS-level dark preference is scoped to html:not([data-theme]), so an explicit user choice always wins
• Sidebar styling — active links use a tinted background + left accent bar; --sidebar-highlight-bg uses native Bulma CSS vars with a bulma-theme('light') reset to stay correct across all theme states

Files changed

File	Change
index.html	Inline script for flash-free early theme apply
src/assets/main.scss	Dark-theme mixin, system-dark scoping, light reset, curtain overlay
src/components/NavBar.vue	Toggle switch UI, theme state management, curtain transition
src/components/ProjectTree.vue	Use color: inherit + --sidebar-highlight-bg for accent bar
src/components/files/CodeEditor.vue	Live Monaco theme switching via useThemeWatch
src/components/files/MilkdownEditor.vue	CSS <link> swap for light/dark Milkdown styles
src/utils/use-theme-watch.ts	New shared composable for observing theme changes
src/utils/index.ts	Re-export useThemeWatch

[markverick]

For #98

[Copilot]

Pull request overview

Adds an app-wide dark/light mode toggle with persistent user preference and live theme updates across the UI and embedded editors, aiming to apply the saved theme before first paint to avoid flashes.

Changes:

• Add early theme hydration in index.html and SCSS theme-variable overrides (including a curtain transition overlay).
• Introduce a shared useThemeWatch utility and wire it into Monaco (CodeEditor) + Milkdown (MilkdownEditor) for live theme switching.
• Update sidebar/NavBar UI and sidebar active-item styling to use a shared highlight CSS variable.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
index.html	Applies saved theme via inline script before Vue mounts.
src/assets/main.scss	Defines theme CSS variables, dark overrides, and the curtain overlay styling.
src/components/NavBar.vue	Adds the theme toggle UI + persistence logic and updates sidebar active styling.
src/components/ProjectTree.vue	Adjusts file tree colors to inherit and uses the shared highlight variable.
src/components/files/CodeEditor.vue	Adds Monaco live theme switching via useThemeWatch.
src/components/files/MilkdownEditor.vue	Switches Milkdown theme CSS via injected <link> and watches theme changes.
src/utils/use-theme-watch.ts	New helper to observe theme changes via data-theme and matchMedia.
src/utils/index.ts	Re-exports useThemeWatch.

Comments suppressed due to low confidence (4)

src/components/NavBar.vue:505

• The theme toggle’s <input> is visually hidden by setting width/height: 0 and opacity: 0, which makes it effectively invisible when focused (keyboard users won’t get a visible focus indicator). Prefer a standard “visually hidden but focusable” pattern (e.g., 1px size + clip/clip-path, or styling the .track on input:focus-visible) so the control remains keyboard-accessible with a visible focus state.

    input {
      position: absolute;
      opacity: 0;
      width: 0;
      height: 0;
    }

src/components/NavBar.vue:18

• The checkbox is controlled by effectiveTheme, but effectiveTheme isn’t updated until after the curtain fade-in finishes. That means the browser will toggle the checkbox on click, then Vue will immediately re-render it back to the old :checked value until the transition completes (visible “snap back” of the knob/icons). Consider updating the reactive theme state immediately (or tracking a pending theme) so the control reflects the user action right away.

        <label
          class="theme-switch"
          :aria-label="`Switch to ${effectiveTheme === 'dark' ? 'light' : 'dark'} mode`"
          :title="`Switch to ${effectiveTheme === 'dark' ? 'light' : 'dark'} mode`"
        >
          <input type="checkbox" :checked="effectiveTheme === 'dark'" @change="toggleTheme" />
          <span class="track">

src/components/NavBar.vue:280

• userTheme is populated via type assertions from localStorage/data-theme, but those sources can contain arbitrary strings at runtime. If an unexpected value is present, effectiveTheme can become invalid and the toggle label/checked logic will be wrong until the user clicks. Validate the stored/attribute value against 'dark' | 'light' (and clear it if invalid) before assigning to userTheme.

const THEME_KEY = 'ownly.theme';
const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
const systemTheme = ref<'dark' | 'light'>(preferredDark?.matches ? 'dark' : 'light');
const userTheme = ref<'dark' | 'light' | null>(
  (globalThis.localStorage?.getItem(THEME_KEY) as 'dark' | 'light' | null) ??
  (document.documentElement.getAttribute('data-theme') as 'dark' | 'light' | null),
);
const effectiveTheme = computed<'dark' | 'light'>(() => userTheme.value ?? systemTheme.value);

index.html:23

• The early theme-apply script sets data-theme to whatever is in localStorage('ownly.theme') without validation. If the stored value is corrupted/unknown, the page can start in an invalid theme state. Consider only applying when the value is exactly 'dark' or 'light' (and otherwise skip/clear it).

      // Apply saved theme before first paint to prevent flash
      (function() {
        try { var t = localStorage.getItem('ownly.theme'); } catch(e) { return; }
        if (t) document.documentElement.setAttribute('data-theme', t);
      })();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

destroy() doesn’t clean up the previous useThemeWatch subscription. Since the watch(() => props.yxml, ...) path calls destroy()/create() without unmounting the component, this will accumulate multiple matchMedia/MutationObserver listeners and can re-inject the theme <link> after teardown. Call unwatchTheme?.() (and null it) inside destroy() (or before destroy() in the watcher) so re-creates don’t leak listeners.

[Copilot]

Theme switching only happens in the fade-in transitionend handler, but the fallback timeout currently only removes the curtain. If the transitionend event never fires (e.g., transitions disabled / duration 0), the theme will never be applied and the toggle becomes a no-op. Apply nextTheme via a timer (and still remove the curtain) or detect zero-duration transitions and commit the theme immediately.

Suggested change

	const fallback = setTimeout(() => curtain.remove(), CURTAIN_TIMEOUT);
	const fallback = setTimeout(() => {
	// Fallback path when no transition events fire: still apply the theme.
	if (document.documentElement.getAttribute('data-theme') !== nextTheme) {
	document.documentElement.setAttribute('data-theme', nextTheme);
	userTheme.value = nextTheme;
	globalThis.localStorage?.setItem(THEME_KEY, nextTheme);
	}
	curtain.remove();
	}, CURTAIN_TIMEOUT);

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

index.html:23

• The early theme-apply script sets data-theme to whatever is in localStorage('ownly.theme') without validating the value. If the stored value is corrupted (or manually edited), the app can end up in an undefined theme state because Bulma’s theme selectors won’t match. Consider whitelisting to 'dark' | 'light' before calling setAttribute (and ignoring/removing any other value).

    <script>
      // Apply saved theme before first paint to prevent flash
      (function() {
        try { var t = localStorage.getItem('ownly.theme'); } catch(e) { return; }
        if (t) document.documentElement.setAttribute('data-theme', t);
      })();

src/components/NavBar.vue:280

• userTheme is initialized by casting the localStorage/data-theme value to 'dark' | 'light' | null without runtime validation. If the stored/attribute value is anything else, effectiveTheme will be inconsistent with the actual document theme and the toggle logic/UI can desync. Recommend normalizing the initial value (accept only 'dark' or 'light', else null) and (optionally) ensuring document.documentElement[data-theme] is set when a valid saved value exists.

const THEME_KEY = 'ownly.theme';
const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
const systemTheme = ref<'dark' | 'light'>(preferredDark?.matches ? 'dark' : 'light');
const userTheme = ref<'dark' | 'light' | null>(
  (globalThis.localStorage?.getItem(THEME_KEY) as 'dark' | 'light' | null) ??
  (document.documentElement.getAttribute('data-theme') as 'dark' | 'light' | null),
);
const effectiveTheme = computed<'dark' | 'light'>(() => userTheme.value ?? systemTheme.value);

src/components/NavBar.vue:549

• The theme toggle hides the checkbox input with width/height: 0 and doesn’t provide any :focus-visible styling on the visible track/knob. This makes keyboard focus hard to see and can hurt accessibility. Suggest keeping the input visually hidden in an accessible way (e.g., off-screen) and/or adding input:focus-visible ~ .track { outline: ... } so focus is clearly indicated.

    input {
      position: absolute;
      opacity: 0;
      width: 0;
      height: 0;
    }

    .track {
      position: relative;
      width: 40px;
      height: 20px;
      border-radius: 10px;
      background: rgba(255, 255, 255, 0.1);
      transition: background 0.25s ease;
      display: flex;
      align-items: center;
      justify-content: space-between;
      padding: 0 5px;
      box-sizing: border-box;
    }

    .track-icon {
      font-size: 10px;
      color: rgba(255, 255, 255, 0.35);
      transition: color 0.25s ease;
      z-index: 1;
    }

    .knob {
      position: absolute;
      top: 2px;
      left: 2px;
      width: 16px;
      height: 16px;
      border-radius: 50%;
      background: rgba(255, 255, 255, 0.5);
      transition: left 0.25s ease, background 0.25s ease;
    }

    input:checked ~ .track .knob {
      left: 22px;
    }

src/assets/main.scss:84

• .theme-curtain is pointer-events: none, so while the curtain is visible (and the UI is obscured) users can still click underlying controls. That can lead to accidental actions during the transition. Consider using pointer-events: auto (at least while .visible is set) to block interaction until the transition completes.

.theme-curtain {
  position: fixed;
  inset: 0;
  z-index: 10000;
  pointer-events: none;
  opacity: 0;
  // background is set inline per-toggle to match the destination theme
  transition: opacity 0.22s ease-in;

  &.visible {
    opacity: 1;
  }

src/components/files/MilkdownEditor.vue:66

• onBeforeUnmount calls unwatchTheme?.() and then destroy(), but destroy() already performs the same unwatchTheme cleanup. This duplication makes the teardown flow harder to follow; consider removing the extra cleanup in onBeforeUnmount and letting destroy() be the single source of teardown logic.

onBeforeUnmount(() => {
  unwatchTheme?.();
  unwatchTheme = null;
  void destroy();
});

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[zjkmxy]

LGTM

[markverick]

There's a visual bug with light-mode splitter (not visible). Will fix in the next commit.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/components/NavBar.vue:315

• preferredDark?.addEventListener('change', ...) only guards matchMedia existence; it will still throw in environments where matchMedia exists but MediaQueryList.addEventListener is not implemented (older Safari uses addListener/removeListener). Consider using preferredDark?.addEventListener?.(...) with a fallback to addListener/removeListener for compatibility.

const THEME_KEY = 'ownly.theme';
const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
const systemTheme = ref<'dark' | 'light'>(preferredDark?.matches ? 'dark' : 'light');
const userTheme = ref<'dark' | 'light' | null>(
  (globalThis.localStorage?.getItem(THEME_KEY) as 'dark' | 'light' | null) ??
  (document.documentElement.getAttribute('data-theme') as 'dark' | 'light' | null),
);
const effectiveTheme = computed<'dark' | 'light'>(() => userTheme.value ?? systemTheme.value);

let interval: ReturnType<typeof setInterval> ;

onMounted(async () => {
  const savedWidth = Number(globalThis.localStorage?.getItem(SIDEBAR_WIDTH_KEY));
  if (Number.isFinite(savedWidth)) {
    sidebarWidth.value = Math.max(SIDEBAR_MIN_WIDTH, Math.min(SIDEBAR_MAX_WIDTH, savedWidth));
  }

  GlobalBus.addListener('project-list', busListeners['project-list']);
  GlobalBus.addListener('project-files', busListeners['project-files']);
  GlobalBus.addListener('chat-channels', busListeners['chat-channels']);
  GlobalBus.addListener('conn-change', busListeners['conn-change']);
  interval = setInterval(() => {
    setNotification();
  },
  250);

  preferredDark?.addEventListener('change', onThemeMediaChange);
});

onUnmounted(() => {
  stopSidebarResize();

  GlobalBus.removeListener('project-list', busListeners['project-list']);
  GlobalBus.removeListener('project-files', busListeners['project-files']);
  GlobalBus.removeListener('chat-channels', busListeners['chat-channels']);
  GlobalBus.removeListener('conn-change', busListeners['conn-change']);
  clearInterval(interval);
  preferredDark?.removeEventListener('change', onThemeMediaChange);
});

function onThemeMediaChange(event: MediaQueryListEvent) {
  systemTheme.value = event.matches ? 'dark' : 'light';
}

src/utils/use-theme-watch.ts:10

• preferredDark?.addEventListener('change', callback) only checks that matchMedia exists; it can still throw if addEventListener is missing on MediaQueryList (older Safari). Use preferredDark?.addEventListener?.('change', ...) and optionally fall back to addListener/removeListener to avoid runtime errors.

  const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
  preferredDark?.addEventListener('change', callback);

src/components/NavBar.vue:297

• Minor formatting issue: let interval: ReturnType<typeof setInterval> ; has an extra space before the semicolon, and the setInterval call is split in a way that’s hard to read. Consider reformatting to the standard setInterval(fn, 250); style for consistency.

let interval: ReturnType<typeof setInterval> ;

onMounted(async () => {
  const savedWidth = Number(globalThis.localStorage?.getItem(SIDEBAR_WIDTH_KEY));
  if (Number.isFinite(savedWidth)) {
    sidebarWidth.value = Math.max(SIDEBAR_MIN_WIDTH, Math.min(SIDEBAR_MAX_WIDTH, savedWidth));
  }

  GlobalBus.addListener('project-list', busListeners['project-list']);
  GlobalBus.addListener('project-files', busListeners['project-files']);
  GlobalBus.addListener('chat-channels', busListeners['chat-channels']);
  GlobalBus.addListener('conn-change', busListeners['conn-change']);
  interval = setInterval(() => {
    setNotification();
  },
  250);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

toggleTheme() removes any existing .theme-curtain, but it doesn’t cancel the previous toggle’s pending fallback setTimeout. If the user toggles twice quickly, the first fallback can still fire later and overwrite data-theme back to the earlier value. Track the active fallback timeout (and/or an incrementing token) and clear/ignore stale timeouts whenever a new toggle starts (or when removing an existing curtain).

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

src/components/files/MilkdownEditor.vue:23

• This file now imports both * as utils and a named useThemeWatch from the same module. Consider using a single import style (e.g., utils.useThemeWatch or named imports only) to keep imports consistent and avoid confusion about where utilities come from.

import { Workspace } from '@/services/workspace';
import * as opfs from '@/services/opfs';
import * as utils from '@/utils';
import { useThemeWatch } from '@/utils';
import type { WorkspaceProj } from '@/services/workspace-proj';

src/components/files/CodeEditor.vue:19

• This file now imports both * as utils and a named useThemeWatch from the same module. Consider using a single import style (either utils.useThemeWatch or switching to named imports) to keep usage consistent.

import * as utils from '@/utils';
import { useThemeWatch } from '@/utils';
import { monacoRegister } from '@/utils/monaco';

src/components/NavBar.vue:280

• localStorage.getItem(THEME_KEY) and the existing data-theme attribute are cast directly to 'dark' | 'light' | null without validating the actual string value. If the stored value is anything else (e.g. stale/corrupted), effectiveTheme can become an unexpected value and html:not([data-theme]) will stop the OS-theme rule from applying. Consider parsing the stored value (only accept 'dark'/'light', otherwise treat as null and optionally remove the invalid localStorage entry / attribute).

const THEME_KEY = 'ownly.theme';
const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
const systemTheme = ref<'dark' | 'light'>(preferredDark?.matches ? 'dark' : 'light');
const userTheme = ref<'dark' | 'light' | null>(
  (globalThis.localStorage?.getItem(THEME_KEY) as 'dark' | 'light' | null) ??
  (document.documentElement.getAttribute('data-theme') as 'dark' | 'light' | null),
);
const effectiveTheme = computed<'dark' | 'light'>(() => userTheme.value ?? systemTheme.value);

src/components/NavBar.vue:321

• commitTheme(theme: string) accepts any string and will set data-theme and persist it. Since only 'dark'/'light' are supported elsewhere, constrain the parameter type (and runtime-validate if needed) to prevent persisting invalid theme names.

function commitTheme(theme: 'dark' | 'light') {
  document.documentElement.setAttribute('data-theme', theme);
  userTheme.value = theme;
  globalThis.localStorage?.setItem(THEME_KEY, theme);
}

src/components/NavBar.vue:515

• The theme toggle’s <input> is visually hidden with width: 0; height: 0; opacity: 0;, which makes the default focus ring invisible for keyboard users. Add a :focus-visible style that targets the visible element (e.g., .track) when the input is focused so the control has a clear focus indicator.

    input {
      position: absolute;
      opacity: 0;
      width: 0;
      height: 0;
    }

src/views/ProjectFileView.vue:876

• color-mix() is used without a fallback. Given the build target includes safari16, older 16.x versions that lack color-mix will ignore the declaration entirely, leaving the element with no background. Consider adding a preceding background: rgba(...) (and box-shadow rgba) as a fallback before the color-mix() declarations.

      color: var(--bulma-text);
      background: color-mix(in srgb, var(--bulma-text) 12%, transparent);
      cursor: pointer;
      z-index: 3;
      transition: background-color 0.12s ease;

      &:hover,
      &:focus-visible {
        background: color-mix(in srgb, var(--bulma-text) 24%, transparent);
      }
    }

    .resizer-grip {
      position: absolute;
      left: 50%;
      transform: translateX(-50%);
      width: 14px;
      height: 22px;
      display: inline-flex;
      align-items: center;
      justify-content: center;
      z-index: 2;
      pointer-events: none;
    }

    .grip-top {
      top: 25%;
      transform: translate(-50%, -50%);
    }

    .grip-bottom {
      top: 75%;
      transform: translate(-50%, -50%);
    }

    .grip-dots {
      width: 4px;
      height: 4px;
      border-radius: 999px;
      background: color-mix(in srgb, var(--bulma-text) 55%, transparent);
      box-shadow:
        0 -6px 0 color-mix(in srgb, var(--bulma-text) 55%, transparent),
        0  6px 0 color-mix(in srgb, var(--bulma-text) 55%, transparent);

index.html:23

• The early theme-apply script sets data-theme to whatever is in localStorage('ownly.theme') without validating it. If the stored value is invalid, it can disable the html:not([data-theme]) OS-theme scoping and leave the app stuck in the wrong theme. Consider only applying the attribute when the value is exactly 'dark' or 'light' (and otherwise clearing it).

    <script>
      // Apply saved theme before first paint to prevent flash
      (function() {
        try { var t = localStorage.getItem('ownly.theme'); } catch(e) { return; }
        if (t) document.documentElement.setAttribute('data-theme', t);
      })();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/utils/use-theme-watch.ts:10

• MediaQueryList doesn't support addEventListener/removeEventListener in some browsers (notably older Safari); in that case this will throw at runtime because optional chaining only checks preferredDark, not the method. Consider using preferredDark?.addEventListener?.('change', ...) with a fallback to preferredDark?.addListener(...)/removeListener(...) for compatibility.

  const preferredDark = globalThis.matchMedia?.('(prefers-color-scheme: dark)');
  preferredDark?.addEventListener('change', callback);

src/components/NavBar.vue:300

• preferredDark?.addEventListener('change', ...) will still throw in browsers where MediaQueryList lacks addEventListener (optional chaining only guards preferredDark). Use preferredDark?.addEventListener?.(...) and/or a fallback to addListener to avoid a runtime crash.

  preferredDark?.addEventListener('change', onThemeMediaChange);
});

src/components/NavBar.vue:311

• On unmount, consider calling cleanupToggle() (or otherwise ensuring any in-flight theme transition is finalized) so the pending setTimeout/transitionend logic can't outlive the component and leave a .theme-curtain in the DOM longer than intended.

onUnmounted(() => {
  stopSidebarResize();

  GlobalBus.removeListener('project-list', busListeners['project-list']);
  GlobalBus.removeListener('project-files', busListeners['project-files']);
  GlobalBus.removeListener('chat-channels', busListeners['chat-channels']);
  GlobalBus.removeListener('conn-change', busListeners['conn-change']);
  clearInterval(interval);
  preferredDark?.removeEventListener('change', onThemeMediaChange);
});

src/components/NavBar.vue:515

• The theme switch <input> is styled to width: 0; height: 0; which can make it difficult/impossible to focus with the keyboard and provides no visible focus indicator. Consider using a standard visually-hidden technique (e.g., 1px size + clip/clip-path) and adding :focus-visible styles on the custom track/knob so keyboard users can operate the toggle.

    input {
      position: absolute;
      opacity: 0;
      width: 0;
      height: 0;
    }

index.html:23

• The early-theme script applies whatever value is in localStorage('ownly.theme') as data-theme without validation. It would be safer to whitelist expected values ('dark'/'light') before setting the attribute so unexpected values don't put the app in an undefined theme state.

    <script>
      // Apply saved theme before first paint to prevent flash
      (function() {
        try { var t = localStorage.getItem('ownly.theme'); } catch(e) { return; }
        if (t) document.documentElement.setAttribute('data-theme', t);
      })();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.