Two related tweaks to the picker topbar/search chrome:
1. The horizontal separators (`.icon-picker-separator` between tabs and
search input, and `.search-section`/`.asset-picker-search` bottom
border) were raw `var(--lx-gray-N, var(--rx-gray-N))` — neutral gray
in OG, out of theme. Inject `--ls-border-color` as the middle step
(#0e5263 in OG, a themed teal that matches the picker hue).
2. Drop the `:focus-within { border-bottom-color: var(--lx-accent-09) }`
accent on `.search-section` and `.asset-picker-search`. The search
input has no validation state to communicate, and the search icon
already brightens on focus (opacity-50 → opacity-75). The blue
accent line was redundant chrome.
Radix themes unaffected (--lx-gray-N still wins step 1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Placeholder fix: previous commit gave placeholder the same themed color
as typed text, collapsing the visual hierarchy. Restore differentiation
by keeping the themed color (--ls-primary-text-color in OG) AND adding
opacity: 0.6 — placeholder recedes against full-strength typed text,
themed in every theme.
(X) clear button fix: bg was raw var(--lx-gray-07, var(--rx-gray-07)),
glyph raw var(--lx-gray-12, var(--rx-gray-12)). In OG both fell to
neutral grays.
- bg → @apply bg-gray-07 (Tailwind chain includes
--ls-quaternary-background-color → #094b5a themed teal in OG)
- hover bg → manual chain with --ls-border-color (#0e5263, one step
lighter; gray-08 chain has no --ls-* middle)
- glyph color → manual chain with --ls-secondary-text-color (#dfdfdf
themed light; gray-12 chain has no --ls-* middle)
Same treatment applied to the asset-picker (X) for consistency. Radix
themes preserved (--lx-gray-N still wins step 1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Section headers, search placeholders, and tab labels rendered as neutral
gray (rx-gray-11/12) in OG because Tailwind's gray-11/gray-12 chains
stop at `--lx-gray-N, --rx-gray-N` (no `--ls-*-text-color` step). On a
teal picker background, neutral gray reads as out-of-theme washout.
Inject `--ls-primary-text-color` (#a4b5b6 in OG — muted teal-gray) as
the themed middle step for label text:
- section-header inline styles (icon.cljs, 8 sites)
- .tab-item resting state (icon.css)
- .ui__input::placeholder for icon & asset pickers
For the brighter active-tab state and its underline, use
`--ls-secondary-text-color` (#dfdfdf in OG) — same role, lighter tone.
Radix themes unaffected: `--lx-gray-N` still wins step 1 of the chain.
Themes without either lx-gray-N or ls-*-text-color (older custom themes)
still fall to neutral `--rx-gray-N` for safety.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The ghost-tile outline still rendered neutral gray (rx-gray-08) in OG
because Tailwind's gray-08 fallback chain is only two steps deep
(`--lx-gray-08 → --rx-gray-08`) — no `--ls-*` intermediate, unlike
gray-01 through gray-07.
Inject `--ls-border-color` manually as the middle step. In OG that
resolves to #0e5263 (a themed teal border, one shade lighter than the
quaternary-teal ghost bg #094b5a), giving a subtle outline that sits
in the same hue family as the tile. Radix themes are unaffected
(--lx-gray-08 still wins step 1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous fallback form `var(--lx-gray-04, var(--rx-gray-04))` only
had two steps, so in OG turquoise (where `--lx-gray-04` is unset) the
ghost tile collapsed to `--rx-gray-04` (neutral hsl(0,0%,15.8%) gray)
on a teal picker background — out of theme harmony.
Switch to `@apply !bg-gray-04`, whose Tailwind chain is three steps
deep: `var(--lx-gray-04, var(--ls-quaternary-background-color, var(--rx-gray-04)))`.
OG's `--ls-quaternary-background-color` (#094b5a, a darker teal)
now wins, and the ghost tile sits in the same color family as the
picker chrome. Radix-* themes are unaffected (their `--lx-gray-04`
still wins step 1).
Outline color (`--lx-gray-08`) keeps the existing 2-step fallback
since Tailwind's chain for gray-08+ doesn't include an `--ls-*`
intermediate. A separate, larger pass would be needed to theme
outlines/foreground grays.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Implements .claude/plans/icon-picker-theme-fit-bulletproof.md. Three
interrelated fixes that together make every picker surface render
correctly across themes (Radix-* and OG turquoise) without breaking
the 9-column grid.
1) Scrollbar opt-out (restores 9-col grid in OG)
- Add `scrollbar-color: auto` to `.cp__emoji-icon-picker > .bd` and
`.asset-picker > .bd`. The OG theme's `:root { scrollbar-color }`
rule sets explicit thumb/track colors, which forces WebKit out of
macOS overlay-scrollbar mode into classic 15px-wide scrollbars.
The 9-px overhead collapsed the row math (9 tiles × 36 + 8 gaps × 4
= 356px) past the available 349px. With `auto` the browser returns
to 6px overlay → 358px available → 9 tiles fit.
2) Theme-adaptive color tokens (fixes pink-section-headers + accent
in selected states)
- 59 picker-scoped `var(--rx-gray-N)` references rewritten to
`var(--lx-gray-N, var(--rx-gray-N))`, matching the existing
fallback pattern used elsewhere in the file. Bare `var(--rx-gray-N)`
was theme-agnostic; the wrapped form lets Radix themes (where
`--lx-gray-N` is set) pick up the themed value while OG keeps the
neutral gray as fallback. Same render in OG today, headroom for
future theme refinements.
- 6 `var(--rx-blue-09)` references in selected states (asset picker
image selection, text picker gallery selection) → `var(--lx-accent-09)`.
Selected items now honor the user's accent in every theme (cyan in
OG, blue/pink/etc. in Radix themes).
- 8 bare `var(--lx-gray-11)` inline styles in icon.cljs rewritten with
the proper fallback. Fixes the pink-section-header bug: when a
color swatch is picked, `.pane-section { color: var(--ls-color-icon-preset) }`
cascades. The bare `var(--lx-gray-11)` was invalid in OG → property
unset → section-header inherited the icon-preset color and turned
pink/green/etc. The fallback keeps the property valid → header
stays themed gray.
3) Structurally borderless edge-to-edge search input
- `.search-section` background switched from raw `var(--lx-gray-03)`
(invalid in OG → transparent) to `@apply bg-gray-03` (Tailwind
utility with full fallback chain through `--ls-tertiary-background-color`).
Section is now themed in every theme.
- `.tabs-section` same treatment.
- `.ui__input` inside `.search-input` / `.asset-picker-search` is now
structurally borderless: `bg-transparent rounded-none`. No more
visible rounded-rect pill; the input is just text + cursor sitting
on the section's themed background. Edge-to-edge feel is now real
(theme-agnostic by construction), not faked via color matching.
- `.search-section:focus-within` and `.asset-picker-search:focus-within`
swap their `border-bottom-color` to `var(--lx-accent-09)` to provide
themed focus indication on the section (the input no longer has its
own focus ring). Matches the CMD-K palette pattern.
Verified live in OG turquoise theme: 9 cols, 1 scrollbar, themed
section bg (#08404f), transparent input, cyan focus border, themed
gray headers regardless of icon-preset color. Verified across All /
Emojis / Icons tabs and the reaction picker.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous single-scroller commit (3364f7bdbf) removed `h-[358px]` from
`.pane-section.has-virtual-list`, assuming `:custom-scroll-parent` alone
would carry the layout. It doesn't: when the popup container provides no
max-height (e.g., reaction picker under a block toolbar / comment thread,
or full picker opened fresh on the Emojis/Icons tab), the chain collapses
to zero — Virtuoso hasn't rendered yet → parent has no content → Virtuoso
measures 0 viewport → renders 0 items → never resolves. The picker shrunk
to ~86px tall with the entire grid missing.
The fixed height and `:custom-scroll-parent` are complementary, not
redundant:
- `h-[358px]` anchors the flex chain so Virtuoso has something to measure
on first render.
- `:custom-scroll-parent` keeps Virtuoso from creating its own scrollbar,
preserving the 9-column grid.
Restoring `h-[358px] overflow-y-visible` with the `searching-result h-auto`
override fixes every surface (All / Emojis / Icons / search / reaction
picker) at 442 picker / 360 .bd / 9 cols / 1 scrollbar.
Also adds defensive `(= :emoji (:type icon))` guards to the two remaining
reaction-picker call sites (block.cljs and comments.cljs) for symmetry
with content.cljs and ui.cljs. Required because comments.cljs didn't yet
require `frontend.handler.notification`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The reaction picker and the full picker's Emojis/Icons tabs all nested
two scrollbars (outer `.bd` + inner Virtuoso scroller). The pair ate
~12px of horizontal space, collapsing the 9-column grid to 8. The
reaction picker additionally rendered a redundant "Emojis · N" header
above its Virtuoso-Header-hosted "Recently used" section, with no
visual separator and the wrong order.
- pane-section: pass `:custom-scroll-parent` to Virtuoso unconditionally
(was gated on `searching?`). Virtuoso defers scrolling to the nearest
`.bd-scroll` ancestor in every mode, so `.bd` is the sole scrollbar.
- icon.css: drop the fixed `h-[358px]` on `.pane-section.has-virtual-list`.
The pane grows to Virtuoso's reported list height; `.bd-scroll` catches
the overflow.
- emojis-cp: render "Recently used" and "Emojis" as sibling pane-sections
(same shape as `all-pane`) instead of nesting recents in Virtuoso's
Header slot. Fixes header ordering and gives the two sections a natural
divider (the Emojis section-header).
- Reaction-picker call sites (block.cljs, comments.cljs, content.cljs,
ui.cljs): replace the inline emoji-only opts with `(merge
reaction-picker-opts ...)` for consistency.
Verified live across every surface (All / Emojis / Icons / Custom /
search / reaction picker): exactly one scrollbar, 9 columns, correct
header order.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add `pt-1` to `.cp__emoji-icon-picker > .bd` so the icon grid (and the
"Recently used" header) doesn't sit flush against the search input.
Asset-picker already had `py-1` on `.bd`, but its avatar-customize
banner negates that margin to keep the expanded-state gradient flush
against the topbar — add a 4px gap to the first `.pane-section`
following the banner so the grid doesn't read crammed against the
banner's bottom border.
Two structurally-distinct dropdown surfaces leak the popup's p-1 padding
in different ways, so they need different fixes:
- Top-level (shui/popup-show! → ui__dropdown-menu-content): icon-search
is a direct child, and its outer class swaps with @*view
(cp__emoji-icon-picker / asset-picker / text-picker). Apply `-m-1` to
all three view classes so switching tabs via "< Back" doesn't jump.
- Sub-content (block-context-menu "Set icon" / "Add reaction"): the
caller wraps the picker in an extra `[:div.p-1]`, stacking two p-1
layers. Pass `{:class "!p-0"}` to the sub-content (matching the
precedent at icon.cljs:4615-4625) and keep the inner wrapper for
breathing room.
Also drop the orphan `.ui__dropdown-menu-sub-content .cp__emoji-icon-picker
{ -m-2 }` rule that would overshoot under the new `!p-0` math.
Reaction pickers (comments + block reactions + editor toolbar +
context menu) only need a search input and an emoji grid — tabs are
redundant (only emoji allowed), color picker is meaningless (emojis
aren't tintable), trash button has no icon to delete.
Two fixes wrapped together:
1. `:tabs [[:emoji ...]]` was unused — `icon-search` filters by
`:allowed-tabs`, not `:tabs`. Switching reaction call sites to
`:allowed-tabs [:emoji]` actually hides Custom/Icons/All tabs
from the strip.
2. Add a `:hide-topbar?` opt. When true, the whole `.tabs-section`
(tab strip + color picker + trash button) and the separator
below collapse — the picker becomes search input + emoji grid.
The invisible `tab-observer` and `keyboard-nav-controller` lift
out of the topbar div so they keep working regardless. The
`.content-pane`'s `role="tabpanel"` + `aria-labelledby` are
gated on topbar visibility — no tab to label means no tabpanel
role.
Reuse the existing-but-orphan `:icon/search-emojis` key (already in
24 language dicts) as the placeholder + aria-label when topbar is
hidden. No new translation work.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three related polish fixes for the icon-picker asset search:
1. Exclude :image from the icon-picker's "Recently used" section. The
data still lives in :ui/ls-icons-used-v2, but rendering :image
alongside icons/emojis creates broken `photo-off` tiles (assets
that were deleted/moved) and conceptually doesn't fit — image
recall has its own home in the asset-picker's "Recently used
assets" row. Filter applied at both render sites: compute-flat-
items and the parallel all-cp path.
2. Drop the `.pane-section` wrapper class from the assets-search-
section. `.pane-section` sets `color: var(--ls-color-icon-preset)`
to tint Tabler SVGs via currentColor — useful for icons/emojis,
but it polluted the image tiles' hover state: the undefined
`--rx-accent-09` border-color rule fell back to currentColor,
which inside .pane-section resolved to the user's picked icon
color. Bypassing .pane-section restores the asset-picker's
neutral cascade.
3. Switch image-asset-item + web-image-item from `border` to
`outline` for hover/focus/selected states. A `border` with
`border-radius` + `overflow-hidden` compressed the inner image's
visible radius below the outer border's curve, making rounded
corners look mismatched between the tile and its image. Outline
draws outside the element without compressing inner content and
follows border-radius automatically, so curves stay consistent
across all shapes (rounded, circle, rounded-rect).
Drive-by: the outline shorthand needs an explicit `currentColor`
fallback inside var() because shorthand parsing fails entirely when
the referenced custom property is undefined (which `--rx-accent-09`
is in this codebase). Without the fallback, outline-style reverts
to its initial `none` and the ring doesn't render at all. The
prior `border-color` longhand didn't have this problem because
longhand var() invalidation falls back to the property's own
initial value (currentColor), which still renders.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The search input already promised "Search emojis, icons, assets" but
only delivered emojis + icons. Add an Assets section to search results
that surfaces local image assets matching the query, so users can pick
an existing image directly from the icon-picker without drilling into
the asset-picker view.
Layout matches the asset-picker exactly: 64x68px tiles in a 5-column
grid, below the Icons section. Section is hidden when no assets match.
Reuses image-asset-item with a new :variant :search opt:
- Tooltip kept (filenames are unreadable on 64px tiles), shortened
to 200ms delay for fast search intent vs 400ms browse intent
- Ghost-retry icon hidden in search context (broken tiles just look
empty rather than show an ugly refresh affordance); the silent
retry logic still runs underneath
- Animation transitions dropped to avoid jank during rapid typing
Implementation notes:
- search-assets filters a per-picker-session cache (::loaded-assets,
loaded once on mount via sync + async paths mirroring the asset-
picker). No DB query on keystroke.
- compute-flat-items adds an Assets section with :cols 5; the existing
move-grid-highlight already supports per-section column counts so
arrow-key nav across 9-col icons → 5-col assets works for free.
- Kbd shortcut extended: Alt+Meta+4 toggles the Assets section
(extending the existing 1/2/3 pattern).
- Avatar-fallback sub-picker opts out via new :no-assets? true — its
job is picking an icon/emoji as a fallback glyph, not an image.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three small fixes from the same review pass:
- colors/bounded-memoize: hits were O(n) because the LRU promote
rebuilt the order vector via (vec (remove …)). Replace LRU with a
plain memo cache that drops half on overflow. Hits become O(1)
map lookup. The working set for adjust-for-contrast and muted-tint
(~200 entries) stays under the 256 cap in normal use, so eviction
is rare and the LRU bookkeeping was pure overhead.
- handler/icon-color: add-recent! and clear! now wrap their
storage writes in try/catch like get-recents already does.
Safari private-browsing and QuotaExceeded would otherwise throw
from React effect cleanups; the recents list is a nice-to-have,
silent drop is the right failure mode.
- ui/tab-items: add :type "button" so a tab can't accidentally
submit a form. Brings parity with the sibling segmented-control
fn. No current call site puts tabs inside a form, but cheap
safety belt.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The "FIXME: somehow those icons don't work" comment hid a real
diagnosis: csk/->Camel_Snake_Case lowercases consecutive caps like
"AB" to "Ab", so the label round-trips back to "IconAb" instead of
the actual "IconAB" tabler export. The renderer's reverse lookup
misses, the icon renders empty.
Document the mechanism, why the filter is correctly scoped to the
three affected names, and why other consecutive-cap exports
(IconEPassport, IconSTurnDown) don't need the same treatment.
The real fix would store the original tabler key alongside the
label so lookup doesn't go through CSK at all, but that's a
rendering-path refactor; the filter is the right local stopgap.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two no-input surfaces in the color picker pane used the same demo hex
(#a1b2c3 / #A1B2C3) — the hex input's placeholder ghost text and
react-colorful's SV pad starting position. Inconsistent casing on
two literals separated by ~60 lines.
Extract as a private constant with a docstring explaining the demo-
value intent. Canonicalize on lowercase to match the rest of the
codebase's hex convention; the input's placeholder renders visually
identical either way since browsers don't case-discriminate hex.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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>
* feat: comments
* comments ux enhancements
* fix: skip comments during cursor navigation
* enhance: support comments on selected blocks
* fix: store comment range targets by uuid
* fix: delete orphaned range comments
* fix: navigate across comments blocks
* refactor: move comments to its own namespaces
* fix lint and tests
* fix: polish block comments
* enhance: add related property to #Comments
* fix: align comments migration test
* fix: non logged in user can't edit or delete comment
* fix: keep comments in place on outdent
* enhance: put comments at top of page
* fix: remove empty comments area
* refactor: split move blocks transaction
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>
