diff options
13 files changed, 201 insertions, 29 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/folder-tree.js b/packages/meshbay-hub/src/meshbay_hub/static/folder-tree.js index 1af6573..6e6dfad 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/folder-tree.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/folder-tree.js @@ -215,8 +215,19 @@ function FolderTreePicker({ } /** - * The button-plus-modal pairing every settings pane wants, so none of them - * has to hold `open` state of its own. + * The chosen folders, as a table, plus the button that opens the picker. + * + * Not a `.settings-row`: that class is `display:flex; justify-content: + * space-between`, so a label, a hint and a value laid out inside one end up + * spread across a single line in whatever order they were written — which is + * how the first version of this read as three unrelated fragments per app. + * + * A table rather than a row of chips because these are lists now. Videos and + * Music can hold several folders, Photos routinely does, and a wrapped run of + * chips gives no column to scan and nowhere to put a per-row control. One + * folder per line, removable where it sits, in the same shape as the shared + * directories table above it — the operator is looking at two lists of + * directories on one page and they should read alike. */ function FolderPickerField({ label, hint, roots, dirs, mode = 'single', requireWritable = false, @@ -226,21 +237,52 @@ function FolderPickerField({ const multi = mode === 'multi'; const chosen = multi ? (value || []) : (value ? [value] : []); + const removeAt = (path) => { + if (!multi) { onChange(''); return; } + onChange(chosen.filter((p) => p !== path)); + }; + return html` - <div class="settings-row"> - <label class="settings-label">${label}</label> - ${hint && html`<p class="settings-hint">${hint}</p>`} - <div class="ftp-field"> - <div class="ftp-field-value"> - ${chosen.length - ? chosen.map((p) => html`<code key=${p} class="ftp-chip">${p}</code>`) - : html`<span class="settings-hint">${t('folder_tree.nothing_selected')}</span>`} - </div> + <div class="folder-field"> + <div class="folder-field-head"> + <h4 class="folder-field-label">${label}</h4> + ${hint && html`<p class="settings-hint">${hint}</p>`} + </div> + + ${chosen.length > 0 && html` + <table class="shared-dirs-tbl folder-field-tbl"> + <tbody> + ${chosen.map((path) => html` + <tr key=${path}> + <td class="sdt-col-dir"> + <span class="sdt-dir-name"> + <${Icon} name="folder" /> + ${path} + </span> + </td> + <td class="sdt-col-actions"> + <button class="sdt-action-btn sdt-action-danger" + disabled=${disabled} + title=${t('folder_tree.remove')} + onClick=${() => removeAt(path)}>\u{2715}</button> + </td> + </tr> + `)} + </tbody> + </table> + `} + + <div class="folder-field-actions"> + ${chosen.length === 0 && html` + <span class="settings-hint">${t('folder_tree.nothing_selected')}</span>`} <button class="btn btn-small btn-secondary" disabled=${disabled} onClick=${() => setOpen(true)}> - <${Icon} name="folder" /> ${t('folder_tree.choose')} + <${Icon} name="folder" /> + ${' '}${chosen.length && multi ? t('folder_tree.add') + : t('folder_tree.choose')} </button> </div> + ${open && html` <${FolderTreePicker} roots=${roots} dirs=${dirs} mode=${mode} diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index edaa70a..2e7d3c5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -821,6 +821,8 @@ export default { 'folder_tree.title_single': 'Ordner auswählen', 'folder_tree.title_multi': 'Ordner auswählen', 'folder_tree.choose': 'Auswählen…', + 'folder_tree.remove': 'Entfernen', + 'folder_tree.add': 'Ordner hinzufügen…', 'folder_tree.confirm': 'Übernehmen', 'folder_tree.expand': 'Aufklappen', 'folder_tree.collapse': 'Zuklappen', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index d1aca93..350a597 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -609,6 +609,8 @@ export default { 'folder_tree.title_single': 'Choose a folder', 'folder_tree.title_multi': 'Choose folders', 'folder_tree.choose': 'Choose…', + 'folder_tree.remove': 'Remove', + 'folder_tree.add': 'Add a folder…', 'folder_tree.confirm': 'Use these', 'folder_tree.expand': 'Expand', 'folder_tree.collapse': 'Collapse', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index 880de2e..22a9461 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -817,6 +817,8 @@ export default { 'folder_tree.title_single': 'Elegir una carpeta', 'folder_tree.title_multi': 'Elegir carpetas', 'folder_tree.choose': 'Elegir…', + 'folder_tree.remove': 'Quitar', + 'folder_tree.add': 'Añadir una carpeta…', 'folder_tree.confirm': 'Usar', 'folder_tree.expand': 'Expandir', 'folder_tree.collapse': 'Contraer', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index eb977f3..3da68b2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -835,6 +835,8 @@ export default { 'folder_tree.title_single': 'Choisir un dossier', 'folder_tree.title_multi': 'Choisir des dossiers', 'folder_tree.choose': 'Choisir…', + 'folder_tree.remove': 'Retirer', + 'folder_tree.add': 'Ajouter un dossier…', 'folder_tree.confirm': 'Utiliser', 'folder_tree.expand': 'Déplier', 'folder_tree.collapse': 'Replier', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index 2f58015..335ed68 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -831,6 +831,8 @@ export default { 'folder_tree.title_single': 'Scegli una cartella', 'folder_tree.title_multi': 'Scegli le cartelle', 'folder_tree.choose': 'Scegli…', + 'folder_tree.remove': 'Rimuovi', + 'folder_tree.add': 'Aggiungi una cartella…', 'folder_tree.confirm': 'Usa', 'folder_tree.expand': 'Espandi', 'folder_tree.collapse': 'Comprimi', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index f22ce21..4dd2f30 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -815,6 +815,8 @@ export default { 'folder_tree.title_single': 'フォルダーを選択', 'folder_tree.title_multi': 'フォルダーを選択', 'folder_tree.choose': '選択…', + 'folder_tree.remove': '外す', + 'folder_tree.add': 'フォルダーを追加…', 'folder_tree.confirm': '決定', 'folder_tree.expand': '展開', 'folder_tree.collapse': '折りたたむ', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 306cf44..cb36d38 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -833,6 +833,8 @@ export default { 'folder_tree.title_single': 'Kies een map', 'folder_tree.title_multi': 'Kies mappen', 'folder_tree.choose': 'Kiezen…', + 'folder_tree.remove': 'Verwijderen', + 'folder_tree.add': 'Map toevoegen…', 'folder_tree.confirm': 'Gebruiken', 'folder_tree.expand': 'Uitklappen', 'folder_tree.collapse': 'Inklappen', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index f2b3faf..91d0d90 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -859,6 +859,8 @@ export default { 'folder_tree.title_single': 'Wybierz folder', 'folder_tree.title_multi': 'Wybierz foldery', 'folder_tree.choose': 'Wybierz…', + 'folder_tree.remove': 'Usuń', + 'folder_tree.add': 'Dodaj folder…', 'folder_tree.confirm': 'Użyj', 'folder_tree.expand': 'Rozwiń', 'folder_tree.collapse': 'Zwiń', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index e608fbd..2b26a0b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -818,6 +818,8 @@ export default { 'folder_tree.title_single': 'Escolher uma pasta', 'folder_tree.title_multi': 'Escolher pastas', 'folder_tree.choose': 'Escolher…', + 'folder_tree.remove': 'Remover', + 'folder_tree.add': 'Adicionar uma pasta…', 'folder_tree.confirm': 'Usar', 'folder_tree.expand': 'Expandir', 'folder_tree.collapse': 'Recolher', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index 68d794e..f1d7f9c 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -802,6 +802,8 @@ export default { 'folder_tree.title_single': '选择文件夹', 'folder_tree.title_multi': '选择文件夹', 'folder_tree.choose': '选择…', + 'folder_tree.remove': '移除', + 'folder_tree.add': '添加文件夹…', 'folder_tree.confirm': '使用', 'folder_tree.expand': '展开', 'folder_tree.collapse': '折叠', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/style.css b/packages/meshbay-hub/src/meshbay_hub/static/style.css index 1354340..0441e14 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/style.css +++ b/packages/meshbay-hub/src/meshbay_hub/static/style.css @@ -24,6 +24,11 @@ --nav-brand: #23b1f0; --sidebar-bg: #f1f5f9; --sidebar-hover: #e2e8f0; + /* Used with a literal fallback in a handful of rules before they were + defined here — same values, so nothing moves; now themeable, and the + names in those rules are real. */ + --warn: #d97706; + --accent-bg: rgba(59, 130, 246, 0.1); --shadow: 0 1px 3px rgba(0, 0, 0, 0.08); --shadow-lg: 0 4px 16px rgba(0, 0, 0, 0.1); } @@ -36,6 +41,8 @@ --text-secondary: #94a3b8; --text-dim: #64748b; --border: #334155; + --warn: #f59e0b; + --accent-bg: rgba(59, 130, 246, 0.18); --error-bg: #450a0a; --nav-bg: #020617; --sidebar-bg: #1e293b; @@ -1910,7 +1917,8 @@ button:disabled { opacity: 0.5; cursor: not-allowed; } white-space: nowrap; } a.transfer-name { - color: var(--link); + /* `--link` is not defined here; the palette's link colour is `--accent`. */ + color: var(--accent); text-decoration: underline; cursor: pointer; } @@ -2138,7 +2146,11 @@ a.transfer-name { position: absolute; top: -6px; right: -8px; - background: var(--danger); + /* `--danger` is not one of this stylesheet's variables — the palette calls + it `--error`. The declaration was invalid, so the unread-count badge had + white text on no background at all. Pre-existing; found while fixing four + more of the same in the folder picker. */ + background: var(--error); color: #fff; font-size: 0.65rem; padding: 1px 5px; @@ -2160,7 +2172,8 @@ a.transfer-name { align-items: center; gap: 8px; padding: 8px 12px; - border-radius: var(--radius); + /* `--radius` was never defined — 8px is what the other cards use. */ + border-radius: 8px; background: var(--bg-surface); margin-bottom: 4px; cursor: pointer; @@ -2170,7 +2183,8 @@ a.transfer-name { background: var(--bg-raised); } .notif-unread { - border-left: 3px solid var(--primary); + /* `--primary` is not defined here either; it is `--accent`. */ + border-left: 3px solid var(--accent); font-weight: 600; } .notif-kind { @@ -2578,7 +2592,7 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } .sdt-add-input { flex: 1 1 auto; min-width: 0; padding: 5px 8px; border: 1px solid var(--border); border-radius: 4px; - background: var(--bg-input, transparent); color: var(--text); + background: var(--bg-surface); color: var(--text); font-family: inherit; font-size: 0.9em; } .sdt-col-toggle { width: 90px; text-align: center; } @@ -2591,9 +2605,9 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } line-height: 1; vertical-align: middle; } .sdt-action-btn + .sdt-action-btn { margin-left: 6px; } -.sdt-action-btn:hover:not(:disabled) { background: var(--bg-hover); color: var(--text); } +.sdt-action-btn:hover:not(:disabled) { background: var(--bg-raised); color: var(--text); } .sdt-action-btn:disabled { opacity: 0.3; cursor: default; } -.sdt-action-danger:hover:not(:disabled) { color: var(--danger, #ef4444); border-color: var(--danger, #ef4444); } +.sdt-action-danger:hover:not(:disabled) { color: var(--error); border-color: var(--error); } .sdt-dir-name { display: inline-flex; align-items: center; gap: 6px; font-weight: 500; } .sdt-dir-name .icon { width: 16px; height: 16px; flex-shrink: 0; } .sdt-row-ejected { opacity: 0.5; } @@ -2603,7 +2617,7 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } padding: 2px 6px; cursor: pointer; font-size: 0.85em; color: var(--text-dim); line-height: 1; } -.root-eject-btn:hover { background: var(--bg-hover); } +.root-eject-btn:hover { background: var(--bg-raised); } .root-eject-btn { margin-left: 8px; } /* An ejected root in the Files table: still listed, deliberately — its files are frozen, not gone — but not somewhere you can walk into. */ @@ -4076,7 +4090,7 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } padding: 16px; } .ftp-panel { - background: var(--bg-panel, var(--bg)); color: var(--text); + background: var(--bg-surface); color: var(--text); border: 1px solid var(--border); border-radius: 8px; width: min(520px, 100%); max-height: min(70vh, 640px); display: flex; flex-direction: column; padding: 16px; @@ -4093,8 +4107,11 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } padding: 4px 0; margin: 8px 0; } .ftp-row { display: flex; align-items: center; gap: 2px; } -.ftp-row.chosen { background: var(--bg-hover); } -.ftp-row.blocked { opacity: 0.4; } +.ftp-row.chosen { background: var(--bg-raised); } +/* 0.4 was low enough that a greyed-out root read as an artefact rather than + a disabled control — the names were barely legible in the screenshot that + prompted this. */ +.ftp-row.blocked { opacity: 0.55; } .ftp-twisty { flex: 0 0 auto; width: 20px; height: 20px; padding: 0; background: none; border: none; color: var(--text-dim); @@ -4111,11 +4128,14 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } .ftp-label .icon { width: 15px; height: 15px; flex-shrink: 0; } .ftp-name { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .ftp-badge { - flex: 0 0 auto; font-size: 0.72em; padding: 1px 5px; border-radius: 3px; - border: 1px solid var(--border); color: var(--text-dim); + flex: 0 0 auto; margin-left: 8px; + font-size: 0.72em; padding: 1px 5px; border-radius: 3px; + border: 1px solid var(--border); background: var(--bg-raised); + color: var(--text-dim); white-space: nowrap; text-transform: uppercase; letter-spacing: 0.03em; } -.ftp-badge.warn { color: var(--danger, #ef4444); border-color: currentColor; } +.ftp-badge.rw { color: var(--accent); border-color: currentColor; } +.ftp-badge.warn { color: var(--error); border-color: currentColor; } .ftp-check { margin-left: auto; flex: 0 0 auto; } .ftp-selection { display: flex; flex-wrap: wrap; gap: 4px; @@ -4123,8 +4143,20 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } } .ftp-chip { font-size: 0.82em; padding: 2px 6px; border-radius: 3px; - background: var(--bg-hover); border: 1px solid var(--border); + background: var(--bg-raised); border: 1px solid var(--border); } .ftp-actions { display: flex; justify-content: flex-end; gap: 8px; } -.ftp-field { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; } -.ftp-field-value { flex: 1 1 200px; display: flex; flex-wrap: wrap; gap: 4px; } +/* An app's chosen folders. Deliberately not a .settings-row: that is a + space-between flex row, and a label, a hint and a list laid out inside one + end up spread across a single line in source order. */ +.folder-field { padding: 8px 0; } +.folder-field-head { margin-bottom: 6px; } +.folder-field-label { + margin: 0 0 2px; font-size: 0.9em; font-weight: 600; color: var(--text); +} +.folder-field-head .settings-hint { margin-bottom: 0; } +/* Borrows .shared-dirs-tbl so the two lists of directories on this page read + as the same kind of thing. */ +.folder-field-tbl { margin-bottom: 8px; } +.folder-field-tbl .sdt-col-dir { font-size: 0.9em; } +.folder-field-actions { display: flex; align-items: center; gap: 10px; } diff --git a/packages/meshbay-hub/tests/test_css_variables.py b/packages/meshbay-hub/tests/test_css_variables.py new file mode 100644 index 0000000..ec576d6 --- /dev/null +++ b/packages/meshbay-hub/tests/test_css_variables.py @@ -0,0 +1,78 @@ +""" +Every `var(--x)` names a variable this stylesheet defines. + +CSS fails silently and generously: an unknown custom property makes the whole +declaration invalid, and the rule around it still applies. So a panel written +`background: var(--bg-panel)` when the palette calls it `--bg-surface` does not +error, does not warn, and does not look obviously wrong in a diff — it just has +no background, and the page shows straight through the modal. + +That is not hypothetical. It shipped in the folder picker, and the same file +already carried one from before: `.notif-badge` asked for `--danger` where the +palette says `--error`, so the unread count was white text on nothing. Found by +a person looking at a screenshot, which is the only thing that was going to +find it. + +A fallback (`var(--x, #ef4444)`) is a lesser version of the same mistake: the +declaration is valid and renders, but the name is still fiction, and the next +reader is told a variable exists that does not. Those are reported separately. + +There was a third check here, comparing the dark palette against the light one +for anything a theme must not inherit. It fired on `--border-focus`, which is +a focus ring deliberately shared by both themes — correct code. A heuristic +that has to be explained away on its first run is worse than no test, so it is +gone rather than exempted. +""" + +import re +from pathlib import Path + +import pytest + +STYLE = (Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" + / "static" / "style.css") + +pytestmark = pytest.mark.skipif(not STYLE.exists(), + reason="the stylesheet is not in this checkout") + +# Where a custom property is defined. Two forms, and both had to be learned +# the hard way while writing this: a scoped one written inline +# (`.video-overview-wrap { --ov-lh: 1.5em; --ov-lines: 3; }`), which an +# anchored pattern misses, and one preceded by an explanatory comment, which a +# `[{;]`-prefixed pattern misses because the character before it is `/`. Either +# mistake reports correct code as broken, which is the fastest way to have a +# test like this ignored. +DEFINE = re.compile(r"(?:^|[{;])\s*(--[A-Za-z0-9_-]+)\s*:", re.M) +# `var(--name` and `var(--name, fallback`. +USE = re.compile(r"var\(\s*(--[A-Za-z0-9_-]+)\s*(,)?") + + +def _text() -> str: + return STYLE.read_text(encoding="utf-8") + + +def test_every_variable_used_without_a_fallback_is_defined(): + source = _text() + defined = set(DEFINE.findall(source)) + assert defined, "no custom properties found — did the palette move?" + + missing = sorted({name for name, fallback in USE.findall(source) + if not fallback and name not in defined}) + assert not missing, ( + "used but never defined, so every declaration naming one of these is " + "invalid and silently does nothing:\n " + "\n ".join(missing)) + + +def test_a_fallback_does_not_excuse_an_unknown_name(): + """ + `var(--danger, #ef4444)` renders, so it is not the same bug — but it is the + same mistake, and it will read as intentional to the next person. Reported + so the name gets corrected rather than the fallback relied on. + """ + source = _text() + defined = set(DEFINE.findall(source)) + guessed = sorted({name for name, fallback in USE.findall(source) + if fallback and name not in defined}) + assert not guessed, ( + "used with a fallback but not defined anywhere — rename to the real " + "variable:\n " + "\n ".join(guessed)) |