The previous fix (61f15f0) restored this.forceUpdate() based on
tracing through preact's diff/component internals, which showed
function components are invoked as c.render(...) - a method call
that should bind `this` to a real Component instance with a working
forceUpdate(). That trace was confirmed against the library source,
but you tested it and the settings list still didn't update in place
after adding a command, so something in that chain isn't panning out
in practice (possibly the setTimeout-based render scheduling, possibly
something else) - not worth continuing to debug blind.
Replacing it with the standard, unambiguous approach: a useState
counter bumped to force a re-render, the documented Preact/React
idiom that doesn't depend on any internal `this` plumbing. Applied to
CommandViewer, MacroViewer, and MobileModifyComponent everywhere they
previously called this.forceUpdate().
Please rebuild and re-test: add/remove/reorder/rename/recolor a
command from the settings list without switching tabs, same for
macros, and the mobile icon-picker modal.
Corrects commit b83e10a. I removed these calls on the assumption
that `this` is undefined in a Preact function component, based on
standard JS semantics. That's wrong for Preact specifically: its
diff implementation invokes function components as `c.render(props,
state, context)` (see node_modules/preact/src/diff/index.js), which
is a method call and therefore binds `this` to Preact's internal
per-instance "PFC backing instance" - a real object with a working
forceUpdate(). So this.forceUpdate() was live code all along, and
removing it broke in-place re-rendering: adding/removing/renaming/
recoloring a command or macro would save correctly but the settings
list wouldn't visually update until you switched tabs and back
(reported by the user after testing).
Restores the calls, now with an explicit `this: { forceUpdate: () =>
void }` parameter on each component function so TypeScript actually
knows the type instead of silently treating it as `any` like the
original code did.
Scope off depend/ban-dependencies for package.json specifically
(JSON has no comment syntax, so an inline eslint-disable isn't
possible here). builtin-modules/node-fetch/npm-run-all are dev-only
build tooling with replacement risk and no end-user impact; tracked
as a follow-up rather than swapped in this branch.
Bare setTimeout/activeWindow.requestAnimationFrame calls should
resolve via window explicitly for popout-window compatibility - if a
command is added from a floating window, the timer needs to run
against that window rather than whatever window happens to be
"active". Affects macro DELAY actions, the page-header button
refresh, and a couple of confetti-timed link opens.
Swaps document.createDocumentFragment() and createEl("div", {...})
for Obsidian's createFragment()/createDiv() equivalents in the
advanced toolbar settings tab. Same DOM output, just going through
Obsidian's own helpers as recommended.
Applies eslint-plugin-obsidianmd's suggested sentence-case casing to
the advanced toolbar settings tab and the macro builder title. None
of this text runs through the t() translation layer (it's plain
literals passed straight to Setting/Notice/createEl), so this only
touches display text, not translation keys - locale files are
untouched.
Worth a glance at the advanced toolbar settings tab and macro builder
modal titles to confirm the wording still reads naturally.
Replaces direct element.style.* assignments with addClass/removeClass
against new dedicated classes in styles.scss:
- menuManager: .cmdr-menu-item for the constant display:flex, and an
.is-visible modifier on .cmdr-menu-more-options replacing the
display none/block toggle (same show-on-hover behavior).
- confirmDeleteModal: .cmdr-confirm-delete-modal for the modal's
z-index.
- chooseCustomNameModal: .cmdr-hide-suggestions and
.cmdr-name-input-wrapper-parent for the two constant display
overrides.
- util.tsx's confetti canvas: all seven style properties were static
(never computed), so they become one .cmdr-confetti-canvas class;
also switched activeDocument.createElement("canvas") to
activeDocument.createEl("canvas", { cls }) since it needed a class
anyway, resolving the prefer-create-el warning on the same line.
Same visual result, just class-driven instead of inline. Worth a
manual check: right-click a command in a context menu (hover reveal
of the "more options" gear), delete a command with confirmation
enabled, use the custom-name modal when adding a command, and trigger
the feedback/donate button confetti in the About tab.
- no-nodejs-modules, no-global-this, and prefer-create-el only exist
to protect the shipped mobile bundle; scope them off for
eslint.config.mts, vitest.config.ts, and src/__tests__/** since none
of that runs inside Obsidian.
- Fix vitest.config.ts's __dirname the same way eslint.config.mts
already was (fileURLToPath(import.meta.url)), since __dirname isn't
defined under ESM.
- Disable no-empty-function (with a description) on the two
intentionally-empty open()/close() mock methods introduced earlier.
- Drop the now-unused `error` catch binding left over from removing
its console.log in an earlier commit.
Preact discards onClick's return value at runtime (unlike React's
synthetic events), so the dozens of existing async onClick={...}
handlers throughout this codebase have always been safe despite
technically mismatching the () => void type React-oriented tooling
expects. Rather than wrap every one in a sync closure, scope
no-misused-promises's checksVoidReturn off for JSX attributes only -
it still catches genuine misuse in forEach/addEventListener/promise
executors, which is where it already caught real issues in the
previous commit.
Fixes the remaining no-floating-promises sites the same way as the
manager files: await inside already-async handlers (confirmDelete
confirmation, settingTab's page-header toggle, commandViewer's
reorder/rename/recolor/mode-change handlers), void for genuine
fire-and-forget (confetti animation, macro command callback,
MacroViewer's save-then-continue calls, sync drag-reorder handlers).
Applies one consistent rule across main.ts and every command manager
(leftRibbon/pageHeader/statusBar/titleBar/explorer/menu):
- Inside an already-async handler (onClick, onclick, event listeners),
await the addCommand/removeCommand/reorder/removeMenu call so
settings are actually saved before the handler resolves, matching
the style already used by the adjacent Change Icon/Rename handlers
in the same files.
- In genuinely fire-and-forget contexts (executeStartupMacros,
forEach over settings on construction), mark the call with `void`
instead, since making the caller async there isn't warranted and
the existing concurrent-fire behavior is intentional.
- Drop `async` from a few addEventListener("transitionend", ...)
callbacks that never awaited anything - the async was doing nothing
but triggering the misused-promises warning.
- executeMacro's COMMAND/LOOP actions no longer await
executeCommandById, since it's synchronous (returns void, not a
Promise) - the await was already a no-op.
- Reject with Error objects instead of plain strings in the three
awaitSelection() modals (addCommand/chooseIcon/chooseCustomName),
since nothing reads the specific string value of these rejections.
No sequencing that previously happened is removed; this only adds
missing awaits/void or removes awaits/async that were already inert.
Worth exercising the affected menus (status bar, title bar, page
header, explorer, left ribbon, right-click "Add/Change Icon/Rename/
Delete") to confirm they still save and update correctly.
Resolves the rest of the no-unsafe-* family by giving TypeScript
enough information instead of suppressing it:
- Extend the Obsidian module augmentation (types.ts) with
App.commands.removeCommand and Vault.getConfig so util.tsx and
main.ts no longer need @ts-ignore around them.
- Cast event.target to the concrete HTMLElement subtype at each
DOM event handler that reads .value/.checked/.offsetWidth
(ChangeableText, MacroBuilder, commandComponent) instead of
@ts-ignore/@ts-expect-error.
- Cast JSON.parse() results to their known shape (MacroBuilder's
macro clone, locales.test.ts's loadJson, main.ts's loadSettings)
instead of letting `any` flow through.
- Drop menuManager's unusual `this: (_item: MenuItem) => void`
parameter and the .call(this, ...) it required - the returned
closure never reads `this`, so this was both unnecessary and,
under this project's non-strict bind/call/apply typing, exactly
why the arguments passed through untyped. Calling the method
directly is equivalent and fully typed.
- Same root cause for MacroBuilderModal's this.close.bind(this) and
executeMacro.test.ts's .bind() - replaced with equivalent
non-bind forms.
- confirmDeleteModal/mobileModifyModal: inline the h(...) call into
render() instead of round-tripping through an intermediately
VNode-typed field, which is where preact's h() lost prop typing.
- vitest.config.ts-style import.meta.dirname swapped for
fileURLToPath(import.meta.url) in eslint.config.mts, since the
project's TS lib doesn't type import.meta.dirname.
Type-level only; no behavior change beyond what's already covered
by the two preceding commits.
The range-input onPointerMove handler compared val (a number) against
target.value (a string), which under strict !== is never equal - so
the dedup check was a no-op and setVal/changeHandler fired on every
pointer move regardless of whether the value actually changed. Parse
to Number first, same as the adjacent ChangeableText handleChange
already does for the same slider.
Worth a manual check: drag the delay/spacing/row-height sliders in
settings and confirm they still track the pointer smoothly.
CommandViewer, MacroViewer, and MobileModifyComponent are all plain
function components (not classes), so `this` inside their handlers
is undefined - this.forceUpdate() has never been able to succeed
since the plugin's first release, it just throws before Preact ever
gets a chance to visually update in place. The actual state mutation
and settings.save() on each of these handlers already happens before
the throwing line, so removing it doesn't change what the UI shows;
it removes a guaranteed, silently-swallowed error on every add/
remove/reorder/rename/recolor action.
Also drops mobileModifyComponent's cmdr-icon-changed listener, whose
only job was calling the broken forceUpdate.
Given this changes real code paths, worth a manual pass: add/remove/
reorder/rename commands and macros from the settings tab, and check
the mobile icon-picker modal, to confirm nothing regresses now that
the throw is gone.
hidingViewer.tsx was already reading .title off left-ribbon items,
but leftRibbonManager.ts compared against a nonexistent .name
property. Combined with the self-compare bug fixed in 8c08808, that
means ribbon-item matching (used to recolor/remove a ribbon icon
when a command is edited or deleted) was effectively matching on
icon alone and never really checking the name/title at all - my
previous fix to i.name === pair.name would have made it match
nothing, since i.name doesn't exist on the real object. This fixes
it to compare i.title === pair.name, and adds a proper `items` type
to the WorkspaceRibbon module augmentation so this is now type-checked
instead of silently `any` behind @ts-expect-error.
Needs a manual check in Obsidian: add a left-ribbon command with a
custom color, confirm the ribbon icon picks up the color; delete a
left-ribbon command, confirm the correct icon (not another one
sharing its icon) is removed from the ribbon.
The "reset to default" button in the advanced toolbar settings calls
ExtraButtonComponent.setTooltip(), which Obsidian only added in
1.4.4. minAppVersion was still declared as 1.4.0, so that call was
already unsupported on 1.4.0-1.4.3 for anyone still on it. This just
makes the declared minimum match reality; 1.4.4 predates essentially
every currently-active install.
The "open-commander-settings" command id predates this lint rule.
Renaming it would silently break any existing user's saved hotkey
binding for opening Commander's settings, so we keep the id and
scope the rule off for src/main.ts instead of renaming or disabling
inline (obsidianmd disallows inline-disabling this rule).
- leftRibbonManager was comparing i.name === i.name (always true)
instead of i.name === pair.name when locating a ribbon item to
recolor/remove, so items sharing an icon with a different-named
command could be matched incorrectly. Now compares against pair.name.
- Replace two stray references to the global `app` with the plugin's
own `app` reference (this.plugin.app / plugin.app), consistent with
the rest of the codebase.
- Use Object.prototype.hasOwnProperty.call() instead of calling
hasOwnProperty directly on values that may not inherit from
Object.prototype.
- Import Command/PluginManifest in types.ts so the Obsidian module
augmentation resolves them instead of relying on ambient globals.
- Drop a leftover console.log of an expected/benign cancellation
error in the "Add command" menu action.
- Add explicit `(): void` return types to several useEffect cleanup
callbacks (settingTabComponent, ColorPicker, AdvancedToolbarSettings,
mobileModifyComponent).
- Widen Mode's literal union with `(string & {})` instead of a bare
string constituent, preserving autocomplete without the redundant
type warning.
- Drop an unnecessary `as ItemView` cast in pageHeaderManager now that
the preceding instanceof guard already narrows the type.
- Switch eslint.config.mts from the deprecated tseslint.config() to
eslint's own defineConfig().
Type-level only, no runtime behavior change.
Adds public/return type annotations and a type-only declare
property (to keep FuzzySuggestModal/SuggestModal's generic
parameter meaningfully typed) so the mock classes satisfy
explicit-member-accessibility, explicit-function-return-type, and
no-unused-vars. No behavior change to the mocks.
Removes eslint-disable comments left over from the pre-flat-config
setup that no longer suppress anything (mostly no-unused-vars on
unused function params, which typescript-eslint doesn't flag by
default) and adds required descriptions to the handful that are
still load-bearing. Also replaces an `as any` cast for the
text-formatting-toolbar third-party API with a locally-typed cast,
since disabling no-explicit-any isn't allowed by the obsidianmd
config. Comment/typing only, no behavior change.
Adds coverage for executeMacro, injectIcons, isModeActive, updateStyles/
removeStyles, and locale completeness, plus the Obsidian API mocks and
setup needed to run them under jsdom. Updates a stale fr.json key to
match the current English source string.
Replaces the legacy .eslintrc/.eslintignore (typescript-eslint 5.x) with a
flat eslint.config.mts on typescript-eslint 8.x, preserving the existing
return-type/accessibility rules and adding the obsidianmd recommended rules.
Also bumps the minimatch override, which was pinned to an old major that's
incompatible with the new ESLint 9 toolchain.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- shell-quote: critical — quote() failed to escape newlines in .op values
- esbuild: high — missing binary integrity verification (bumped to 0.28.1)
- Fix two return type errors in MacroBuilder.tsx exposed by stricter tsc
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove unused imports (getIconIds, setIcon, exp, arrayMoveMutable, etc.)
- Add missing explicit return types on functions and arrow functions
- Add public accessibility modifier to PageHeaderManager.buttons
- Fix mixed spaces/tabs in constants.ts and commandComponent.tsx
- Wrap case block declaration in MacroBuilder.tsx to fix no-case-declarations
- Add eslint-disable comments for false-positive no-unused-vars on
function type annotation parameter names and TypeScript enum members
- Remove dead forceUpdate reference in MacroBuilder.tsx (was referencing
this in a function component)
Reduces from 64 errors + 2 warnings to 0 errors + 2 warnings (the remaining
warnings are intentional non-null assertions).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Commander now detects the standalone Text Toolbar plugin at load time and:
- Pushes saved commands into the toolbar via its public API (setCommands)
- Shows a "Text Toolbar" tab in Commander settings when the plugin is installed,
using the existing CommandViewer UI for add/remove/reorder
The tab is hidden when the Text Toolbar plugin is not installed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The text toolbar feature has been moved to the standalone obsidian-text-toolbar
plugin at plugins/text-toolbar. This commit removes leftover staging artifacts.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Set icon directly on addCommand so it appears in the command palette,
status bar, and mobile toolbar. Also fixes removeCommand referencing
bare `app` instead of `plugin.app`, and replaces Array.remove() during
forEach iteration (mutation hazard) with a filter in injectIcons.
Fixes#149 (cherry-picked from PR by Anton/Lyqed)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixed bug where status bar commands would be deleted from the list when
the command stopped being available (e.g. from other plugins that hadn't
loaded yet at onLayoutReady time).
Co-authored-by: davidvkimball <davidvkimball@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
esbuild-sass-plugin@^3.3.1 resolved to 3.7.0 in CI, which requires
esbuild>=0.27.3. Our esbuild is ^0.25.0, causing ERESOLVE. Pinning
to exactly 3.3.1 keeps compatibility with esbuild 0.25.x.
Fixes moderate severity Dependabot alert:
- esbuild <=0.24.2: dev server could be queried by any website
(GHSA-67mh-4wv8-2f99)
Changes:
- esbuild: ^0.15.16 → ^0.25.0
- esbuild-sass-plugin: ^2.4.2 → ^3.3.1 (required for esbuild compat)
- sass-embedded: ^1.71.1 added (peer dep for esbuild-sass-plugin 3.x)
- scripts/esbuild.config.mjs: replaced removed watch:boolean option with
esbuild.context() + ctx.watch() API (changed in esbuild 0.17)
All changes are dev/build toolchain only and do not affect the
compiled main.js shipped to users.
