Two related cleanups for the lightbox extension.
1. Rapid double-click guard. preview-images! did not check whether a
lightbox was already open before creating a new one. Two synchronous
clicks each created a lightbox + attached window listeners, then the
second assignment to window.photoLightbox orphaned the first instance
— its swallow filter and Escape handler stayed bound, soft-breaking
outside clicks until reload. Early-return if pswp is active.
2. Docstring fix. Previous wording claimed the Escape handler is
"identical to PhotoSwipe's own" when no popper is open. It isn't —
it stopImmediatePropagation's first, which preempts other global
Escape hooks (notably :editor/escape-editing). That behavior is
actually desired so closing a preview doesn't also exit block-edit
mode. Reword to match reality.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
shui/avatar-image was called without an :alt prop, which leaves the
underlying <img> with no alt attribute — some screen readers fall
back to announcing the URL or filename.
The avatar is always rendered next to its label (page/block title),
which the screen reader already announces. Passing the title as alt
would double-announce; the right call is explicit empty alt to mark
the image as decorative per WCAG.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The tab strip already had role=tab + aria-selected + roving tabindex,
but the content area lacked role=tabpanel and ids, and the tabs lacked
aria-controls. Screen readers couldn't associate the panel with the
active tab label.
ui/tab-items: add :tab-id-prefix and :panel-id opts. When set, each
button emits id=\"<prefix>-tab-<id>\" and aria-controls=<panel-id>.
Defaults to nil so other callers (none today) stay byte-identical.
icon-search panel: id=\"icon-picker-panel\", role=\"tabpanel\",
aria-labelledby pointing at the active tab's id (dynamic via @*tab).
No tabindex=0 on the panel — its content always has focusable
descendants so an extra stop would be redundant per APG.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The cond that strips a picker-emitted icon down to persistable fields was
duplicated across block.cljs (page-title), property/value.cljs (default-
icon-row), and views.cljs (table row picker). Three near-identical
branches in three places.
Extract to `icon-component/icon-data-for-storage` — a pure fn paralleling
`normalize-icon` but for write paths instead of render paths. Each call
site collapses to a single function call.
Drive-by: property/value.cljs's image branch previously dropped `:id`
where block.cljs kept it. The unified helper uses the more complete
shape, which is harmless for existing image storage (extra field) and
matches what the other write paths emit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The renderer's fire-and-forget <unlink-asset hook fires for any tx
carrying deleted-assets, including:
- bulk graph sync downloads (:sync-download-graph?) — another device's
retraction would wipe the local copy of an asset we may still want
- RTC remote applies (:rtc-tx?, :rtc-download-graph?)
- undo/redo replays (:undo?, :redo?)
- multi-tab fan-out (every connected tab's renderer received the same
deleted-assets payload and raced to unlink the same path)
Add a tx-meta gate so the unlink only fires when the deletion is a
fresh, local user action. The bulk-sync case is the load-bearing one
(real data-loss risk). The undo/redo flags match codebase convention
elsewhere but note that real undo-preserves-asset support needs a
trash-folder pattern, not just a gate — flagged with a TODO.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The per-block doseq path (avatar/text/clear) used to issue N independent
transactions through the worker — partial mid-loop failures left the
selection in a mixed state, no shared undo step, and N reactive re-renders.
Wrap the doseq in one ui-outliner-tx/transact! with op-tag
:set-block-properties. The inner handler calls each open their own
transact! but short-circuit when an outer binding is active (per the
macro at frontend/modules/outliner/ui.cljc), so all N ops collapse into a
single atomic DB transaction, one undo step, and one re-render cascade.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The per-block icon-data builder explicitly select-keys'd only colors,
which silently reverted rounded-rect avatars to circles and dropped
text alignment/mode choices when applying an icon to multiple selected
blocks. Add the missing keys and rename the local from `colors` to
`styling` to match the broader intent.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Section-collapse shortcut now uses util/meta-key? so Ctrl+Alt+1/2/3
works on Win/Linux (Mac unchanged: ⌥⌘1/2/3).
- Move `delete-mode (if del-btn? :remove :hidden)` default out of `:or`
and into the let body in asset-picker and text-picker — CLJS evaluates
:or defaults in the outer scope, so the sibling `del-btn?` reference
was an undeclared var.
- Add ^:large-vars/cleanup-todo to asset-picker for parity with
icon-search.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
When an entity inherits a class default-icon but has no own override,
the trash button now writes `{:type :none}` directly with one click,
labelled "Hide inherited icon". Recovery is via the page-title Restore
affordance. Previously the trash button disappeared in this state,
leaving no way to opt out of inheritance.
Also fixes the icon-picker on-chosen wrapper to be variadic and forward
the action keyword — without this, asset-picker / text-picker delete
clicks silently dropped `:remove-entirely` and the entity wrote `nil`
instead of `:none`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plan v2 specified all three trash sites should share the cond/dropdown
pattern driven by delete-mode. The first implementation only updated
the outer icon-search trash (icon.cljs:7155); the inner asset-picker
(icon.cljs:3887) and text-picker (icon.cljs:6334) kept the
single-click behavior, so users who opened the picker on an avatar/
image icon (which auto-routes to asset-picker) couldn't reach the
"Remove entirely" option.
Changes:
- Add `delete-mode` to asset-picker and text-picker prop destructuring
(with :or default `(if del-btn? :remove :hidden)` for back-compat).
- Replace `(when del-btn? (shui/button …))` with the same cond block
used at the outer trash — :hidden, :remove, :two-option branches.
- Thread `:delete-mode delete-mode` from icon-search to both sub-pickers.
- Change the sub-picker `:on-delete` shims to 1-arg `(fn [& [action]] …)`
so the action keyword chosen in the sub-picker's own dropdown flows
through to the parent on-chosen.
Verified live: open picker on a tagged page with class default-icon +
own override (Dr. Robert Whitfield #Person) → asset-picker view opens
→ trash button has aria-haspopup="menu" and tooltip "Remove icon…" →
real mouse click opens dropdown with [↩ Revert to default] / [🗑 Remove
entirely].
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The trash-button render switched the picker render path between
:hidden, :remove, and :two-option modes via (case delete-mode ...).
This caused an "Objects are not valid as a React child" runtime error
on icon-free pages — even though :hidden branch returned nil, the
parent dialog's render still threw.
Reproduced bisect: same logic with (cond (= delete-mode :hidden) ...)
renders cleanly. The case form alone — with keyword tests and a Radix
dropdown in one branch — leaked a keyword into React's child tree.
Swap to `cond` and add a code comment explaining the gotcha so a
future cleanup doesn't re-introduce the case form.
Verified live: Add icon on the icon-free "wowie" page now opens the
picker cleanly with no error boundary.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The icon-picker's "Remove entirely" dropdown option writes
{:type :none} to suppress inheritance. Without a counterpart UI, users
had no way back — the trash button hides itself when the entity is
already suppressed.
In page.cljs:242's db-page-title-actions, detect the :none sentinel
and swap the affordance:
- Entity has no icon (nil / unresolvable): label "Add icon" → opens picker
- Entity is suppressed via :type :none: label "Restore icon" with ↩ prefix → one-click retract
Single affordance slot, contextual behavior. The label change makes
the user's situation legible: they see WHAT will happen ("Restore",
not "Add") and can act with one click — no picker round-trip needed.
Verified live: page "yolo" was at :type :none. After reload, affordance
label reads "Restore icon" with arrow-back-up prefix. Click → property
retracted → label flips to "Add icon".
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Implements the new delete UX per .claude/plans/delete-button-bulletproof.md.
In icon-search at line 7023, the trash button now renders conditionally
on `delete-mode`:
- :hidden — not rendered
- :remove — single-click immediate retract (today's behavior)
- :two-option — opens a Radix dropdown with two labeled items:
[↩ Revert to default] → retract :logseq.property/icon
[🗑 Remove entirely] → write {:type :none} sentinel
Trash glyph stays consistent in every mode that shows it; the `…`
suffix in the tooltip + aria-haspopup="menu" signal the menu without a
chevron. Dropdown items have icon prefixes + text labels.
`on-chosen` extended to a 3-arity `(e icon-value action)` where action
is :remove | :revert | :remove-entirely | nil. The page-title on-chosen
in block.cljs:3964 branches:
- :remove-entirely → writes :type :none (suppresses inheritance)
- :revert / :remove / nil → retracts (lets inheritance resume)
Existing single-arg `(on-chosen nil)` callers still work via variadic
tail. Asset-picker and text-picker trash shims forward :revert when
delete-mode is :two-option (their trash stays single-click; revert is
the safe default that lets class default reappear on tagged pages).
Verified live: outer picker on Dr. Robert Whitfield (tagged #Person
with class default-icon avatar):
- Click trash → dropdown opens with both items
- Choose Revert → photo override retracted, class default avatar renders
- aria-haspopup="menu", tooltip "Remove icon…"
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Classify the picker's delete affordance into :remove | :two-option |
:hidden based on entity state + property scope:
:two-option — has own icon AND class inheritance would kick in
:remove — has own icon, no inheritance source
:hidden — no own icon, OR entity is :type :none (suppressed)
Reads :block/tags + :logseq.property.class/default-icon so the picker
re-renders when a class's default-icon changes (db-mixins/query
registers them as deps).
Bind a reactive `delete-mode` alongside the existing `del-btn?` in
icon-search. del-btn? becomes derived from delete-mode for back-compat
with downstream callers; delete-mode will drive the dropdown render in
the next commit.
No UI change yet — this is the pure data layer.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Trash clicks left optimistic atoms intact:
- asset-picker's ::pending-icon could be re-injected by an in-flight
<save-image-asset! resolving AFTER delete, leaving a phantom
image-placeholder
- :ui/icon-hover-preview lingered as a ghost overlay
- ::asset-picker-initial-mode persisted, pinning the next picker
reopen to the deleted icon's mode
- *upload-status banner stayed visible mid-upload deletes
Add a reset-picker-transient-state! helper near the preview helpers in
icon.cljs and wire it into all four trash sites:
- asset-picker (icon.cljs:3882, inside the asset-picker component) —
clears its own *pending-icon and *upload-status
- icon-search shims at :6788 and :6821 (delegated from inner pickers)
and outer trash at :7027 — clear *asset-picker-initial-mode + the
global hover-preview
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
block.cljs:3922's `or` chain treated `{:type :none}` as truthy, so the
.ls-page-icon div was rendered with an empty shui ghost button inside
(the icon renderer at icon.cljs:638+ has no `:type :none` branch and
returns nil). Visible as a gray 38×38 rounded box next to the page title
after the user deleted an icon, preventing the title from flowing
flush-left.
Filter the user-data branches (own, inherited-default-icon, tag icons)
through icon-component/renderable-icon? so `{:type :none}` and other
unrenderable values produce nil and the slot is omitted entirely. The
hardcoded class/property fallbacks (`{:type :tabler-icon :id "hash"}`,
`…"letter-p"}`) bypass the predicate since they are guaranteed
renderable once `js/tablerIcons` loads.
Belt-and-suspenders: also defends against legacy `:type :none` values
already persisted in user DBs from prior deletes.
Verified live: page with persisted `:type :none` now renders title
flush-left, no .ls-page-icon div in the DOM.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
icon-row was modified earlier in this branch (c85e8e5588) to add
< rum/reactive db-mixins/query mixins so a model/sub-block call could
refresh the entity reactively. Those mixins turn rum/defc into a class
component, but the component still called hooks/use-effect! — which
internally calls React.useEffect and is only valid inside a function
component's body. The result: an Invalid-hook-call exception every time
icon-row rendered, surfacing as a "Something wrong, please try again"
toast when the user clicked to add a page icon.
Convert rum/defc → rum/defcs (class component, takes state as first
arg) and replace the hooks/use-effect! cleanup with a :will-unmount
lifecycle. The original effect returned a cleanup that restored the
cursor when `editing?` was true on unmount; lifecycle-based unmount is
the natural equivalent.
Verified live: page reloads, picker can render icon-row without
throwing — zero hook errors in console.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Five defonce atoms (*drag-active? *drag-depth *asset-picker-open?
*upload-status *uploading-files) were module-global. Two consequences:
1. Two pickers open simultaneously (sidebar + main view) cross-talked —
drag highlights, upload progress, and the open flag were shared.
2. The OUTER icon-search root and the INNER asset-picker each have their
own drop zone, but they collided on the SAME *drag-active? / *drag-depth
even within a single picker session.
Move state into rum/local on each owning component:
- asset-picker (inner): 4 locals — drag-active?, drag-depth,
asset-picker-open?, upload-status. Aliased in the main let-binding so
existing *foo references resolve without code-site edits; lifecycle
hooks (:did-mount, :will-unmount) thread state explicitly.
- icon-search (outer): 2 locals — drag-active?, drag-depth — independent
from the inner picker's so nested drop zones no longer collide.
Also dropped *uploading-files entirely — it was declared but never read
or written anywhere in the file.
Compile clean. Page renders without runtime errors. Live drag-drop verify
deferred to manual testing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pipeline ran (take 24) BEFORE (filter should-keep?), so an existing
duplicate at position 24+ in the recents list was truncated unfiltered
instead of caught by the dedup. Then (cons normalized) prepended the
new pick, producing 25 items — both violating the 24-item cap and
failing to actually move the dup to the front.
Reorder: filter across the whole existing list, cons the new pick,
THEN cap at 24. REPL-verified on a 27-item list with the duplicate at
index 24: old pipeline stored 25 items (cap violated), new pipeline
stored 24 (cap honored, dup correctly displaced by the cons).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Logseq convention is to log via lambdaisland.glogi (already used across
handler/, worker/, etc.). Four console.error sites in icon.cljs ported
to log/error with structured event keywords and context maps:
- :icon/url-asset-fetch-failed {:url :error}
- :icon/url-asset-ipc-failed {:url :error}
- :icon/grid-subvec-failed {:start :end :count :error}
- :icon/clipboard-paste-failed {:error}
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Logseq convention: function names prefixed with `<` return a promise.
save-image-asset! uses p/let internally and returns the entity promise;
all three external callers wrap it in p/let / p/all. Sibling
<save-url-asset! already follows the convention. Rename for consistency.
Five occurrences updated (defn, internal arity dispatch, 3 callers).
Compile clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The flag was introduced upstream by Tienson Qin (4c6c3322fe, Nov 2025,
"enhance(ux): show node icon in search results") to make cmdk skip the
block's own :logseq.property/icon and fall through to the class's icon
or fallback, keeping search rows visually consistent at the class level.
During this branch's icon-picker refactor, get-node-icon was rewritten
to a new own → tag-default → type-default inheritance model and lost
the opts map entirely. The four cmdk callers kept passing
{:ignore-current-icon? true} but get-node-icon-cp silently ignores it.
The new behavior (show the block's effective icon with full inheritance)
is preferable for the new picker model — pages with meaningful per-
instance icons like photos show their photos in cmdk, instead of a
generic class glyph. Drop the dead flags to reflect actual behavior and
remove the misleading vestige. If "always show class default in cmdk"
is wanted later, it's a deliberate new feature, not a regression to
restore.
Verified live: cmdk renders page rows with their inherited/own avatars,
asset rows with asset glyphs, block rows with dot fallbacks.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
block-title-with-icon had zero callers — the editor.cljs and
property/value.cljs sites that used to compose via this helper were
migrated to the explicit 20×20 icon-slot pattern (single icon per row,
sourced from get-node-icon-cp), and CMD-K never composed through it.
The only remaining references were explanatory comments documenting
*why* the new pattern doesn't reuse it.
Delete the function and rewrite the four comments to describe the
de-dup rule directly (instance rows that diverge from a class default
would otherwise render the same icon twice) rather than referencing a
dead symbol that would rot future readers.
Compile clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
state.cljs and theme.cljs both stamped data-theme + body classes onto
the DOM (in apply-theme-to-dom! and the container's mount effect
respectively). The duplicate setAttribute work is idempotent in
practice but the two code paths drift on edge cases (plugin hook
order, custom-theme application) and obscure who owns the stamp.
Promote state/apply-theme-to-dom! to public and have theme.cljs's
effect call it instead of inlining its own copy. Now: set-theme-mode!
calls it synchronously before set-state! (so subscribers see fresh
DOM on next render); the container effect calls it on mount and on
every theme prop change (so the initial render and any external
:ui/theme writes also stamp the DOM). Effect retains its
custom-theme + plugin-hook + re-render-root! responsibilities.
re-render-root! kept on toggle: bg-var cache invalidation (commit
6b4e5fb910) makes recomputed values correct, but components that
don't subscribe to :ui/theme but DO render avatar/contrast colors
would keep stale inline styles without a forced reconciliation.
Verified live: set-theme-mode! "dark" → data-theme stamped to "dark",
`dark` class added to <html>, body classes updated, end state
consistent.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
read-bg-var was called on every render of every colored icon via three
call sites in icon.cljs (avatar-fallback-style, icon color? wrapper,
get-node-icon-cp). On a graph with many tagged pages — page list, cmdk
results, sidebar — that meant hundreds of synchronous getComputedStyle
calls per frame, each forcing a style recalc / layout flush.
Cache values per CSS var name in a defonce atom; a MutationObserver on
documentElement watching data-theme + class invalidates the cache on
theme flips (apply-theme-to-dom! writes both). Observer callbacks fire
at the microtask boundary — before React commits the render scheduled
by the same set-state! that triggered the flip — so subscribers always
see fresh values on the next paint without an explicit invalidation
call from state.cljs.
Cache also holds nil so unset vars don't repeatedly hit getComputedStyle.
Verified live:
- 1001 read-bg-var calls after initial fill: 0 getComputedStyle calls.
- Flip documentElement data-theme attribute: exactly 1 fresh
getComputedStyle call on the next lookup (observer fired, cache
cleared, value recomputed).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
attach! sets every Radix popper to inert=true and installs 5 window-
capture listeners (pointerdown/mousedown/click/contextmenu/keydown).
Cleanup was wired only to PhotoSwipe's "destroy" event, but if .init
or .loadAndOpen threw, that event never fired — leaving the listeners
attached and every popper permanently inert (soft-bricked app).
Wrap init+loadAndOpen in try/catch: on failure synchronously call
detach! to roll back inert + remove listeners, clear the window-global
so mobile/navigation's later .destroy call doesn't act on a broken
instance, and rethrow so the underlying error surfaces.
Verified by stubbing PhotoSwipeLightbox to throw inside .init and
calling preview-images!: caught exception bubbles up, inert restored
to false on a probe popper, and window.photoLightbox is null.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Cleanup pass following the icon-picker i18n migration. Removes 84 keys
in :settings-page/*, :content/*, :linked-references/*, :text/* that
were imported from a sibling branch but have no (t ...) call sites in
the current codebase, plus 5 stale bare-form shortcut keys
(:editor/copy, :dev/show-{block-data,block-ast,page-data},
:asset/confirm-delete) whose dynamic shortcut-desc lookup goes through
the decorated form :command.<ns>/<leaf> instead.
Verified via:
- ripgrep across src/ and deps/ for each key (no static call sites)
- inspection of shortcut-desc-by-id + decorate-namespace path
- bb lang:validate-translations: down from 94 unused → 5 unused
- 5 remaining unused are master-owned :icon/icons-count,
:icon/matched-count, :icon/search-{all,emojis,icons}; left in place
- bb lang:lint-hardcoded -w: still 0 hardcoded strings
- shadow/compile :app: 0 warnings
Keys existed only in en.edn — no other locale files affected.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move icon picker + property default-icon-row + selection action-bar text
through frontend.context.i18n/t per .agents/skills/logseq-i18n. Adds
~93 new keys across :icon/*, :icon.{asset,asset-mode,asset-search,
avatar-band,avatar-fallback,avatar-scope,avatar-tab,clipboard,color,
fallback,mode,section-header,shape,text-picker,text-tab,upload,
web-images}/*, :class.default-icon/* namespaces. Each new key is
defined in both en.edn (source) and zh-cn.edn (required second locale
per skill Rule 3).
Compound sentences kept whole via interpolate-rich-text-node (Rule 4);
dynamic-count messages use Tongue function values (Rule 6); existing
generic keys reused where semantics matched (:ui/cancel, :ui/reset,
:ui/empty, :ui/untitled, :property.status/done, :context-menu/set-icon).
Verified:
- bb lang:lint-hardcoded -w → 0 hardcoded strings
- bb lang:validate-translations → all referenced keys defined
- shadow/compile :app → 0 warnings
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Brings 99 new master commits forward. 5 file-level conflicts resolved:
- deps/db/src/logseq/db/frontend/schema.cljs — bumped version to "65.32"
to accommodate both sides' migration chains
- src/main/frontend/worker/db/migrate.cljs — both branches defined a
65.26 migration; renumbered master's :logseq.property.repeat/repeat-type
to 65.32 after my 65.26..65.31 chain
- src/main/frontend/components/content.cljs — dropped master's debug
prn (commit "remove debug print"); kept HEAD's indentation
- src/main/frontend/components/icon.cljs — kept HEAD's refactor of
emoji-cp into type-dispatched emoji/icon/text/avatar components
+ render-item dispatcher; master's simpler emoji-cp would break
the item-render call site
- .agents/skills/logseq-task-on-lambda/SKILL.md — took master's
updated skill doc verbatim (not my work area)
Re-added /.claude/plans/ to .gitignore after auto-merge picked
master's rewrite of /.claude → /.claude/settings.local.json and
dropped my prior ignore rule.
Compile verified clean against shadow-cljs :app and :test builds
(0 new warnings). App loads in browser without runtime errors.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Breadcrumb segments in CMD-K rendered the page's full 20px avatar above
12px breadcrumb text, breaking the line box, competing with the row's
own 20px icon for attention, and producing a "double face" effect when
the same entity appeared as a row icon and a breadcrumb icon nearby.
Hide the segment icon behind the existing .breadcrumb--search-result
modifier so all other surfaces (app header, right sidebar, FSRS, etc.)
keep their breadcrumb icons unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
