clone/logseq
1
0
Fork 0
mirror of https://github.com/logseq/logseq.git synced 2026-05-19 18:32:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address remaining should-fix items from pr-review

Browse Source 
- Extract animate-element!/highlight-row! helpers in shortcut-press! (was 4x duplication)
- Extract matches-keystroke? predicate (was duplicated in count + render)
- Replace <a> tags with <button> for all interactive elements (remove, clear, reset, undo, toggle, refresh)
- Add aria-label to icon-only buttons for screen reader support
- Add button reset CSS for elements changed from <a> to <button>
- Replace hardcoded rgba colors in CSS glow/shadow/row-pressed with theme variables
- Define press-animation-ms constant (was magic number 160)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
scheinriese
2026-03-10 11:17:57 +01:00
committed by Tienson Qin
parent 8048c30bca
commit 4c5b7ca511
4 changed files with 94 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
deps/shui/src/logseq/shui/shortcut.cljs vendored
View File

@@ -149,11 +149,33 @@
(string/split % #"\+")
%)))))))
(def ^:private press-animation-ms 160)
(defn- highlight-row!
"Add or remove the row highlight class on the closest .shui-shortcut-row ancestor."
[^js container add?]
(when-let [^js row (or (.closest container ".shui-shortcut-row")
(.-parentElement container))]
(if add?
(.add (.-classList row) "shui-shortcut-row--pressed")
(.remove (.-classList row) "shui-shortcut-row--pressed"))))
(defn- animate-element!
"Add pressed class, optionally highlight row, then auto-reset after animation."
[^js el ^js container highlight-row?]
(.add (.-classList el) "shui-shortcut-key-pressed")
(when highlight-row? (highlight-row! container true))
(js/setTimeout
(fn []
(.remove (.-classList el) "shui-shortcut-key-pressed")
(when highlight-row? (highlight-row! container false)))
press-animation-ms))
(defn shortcut-press!
"Central helper to trigger key press animation.
Finds all nodes with matching data-shortcut-binding and animates individual keys.
Optionally highlights parent row.
Args:
- binding: normalized shortcut binding string (e.g., \"cmd+k\")
- highlight-row?: if true, also highlights parent row (default: false)"
@@ -163,44 +185,11 @@
selector (str "[data-shortcut-binding=\"" normalized "\"]")
containers (.querySelectorAll js/document selector)]
(doseq [^js container (array-seq containers)]
;; Find all individual keys within this container
(let [keys (.querySelectorAll container "kbd.shui-shortcut-key")]
(if (> (.-length keys) 0)
;; Animate individual keys
(doseq [^js key-el (array-seq keys)]
(.add (.-classList key-el) "shui-shortcut-key-pressed")
(when highlight-row?
(let [^js row (or (.closest container ".shui-shortcut-row")
(.-parentElement container))]
(when row
(.add (.-classList row) "shui-shortcut-row--pressed"))))
;; Auto-reset after animation duration
(js/setTimeout
(fn []
(.remove (.-classList key-el) "shui-shortcut-key-pressed")
(when highlight-row?
(let [^js row (or (.closest container ".shui-shortcut-row")
(.-parentElement container))]
(when row
(.remove (.-classList row) "shui-shortcut-row--pressed")))))
160))
;; Fallback: animate container if no keys found
(do
(.add (.-classList container) "shui-shortcut-key-pressed")
(when highlight-row?
(let [^js row (or (.closest container ".shui-shortcut-row")
(.-parentElement container))]
(when row
(.add (.-classList row) "shui-shortcut-row--pressed"))))
(js/setTimeout
(fn []
(.remove (.-classList container) "shui-shortcut-key-pressed")
(when highlight-row?
(let [^js row (or (.closest container ".shui-shortcut-row")
(.-parentElement container))]
(when row
(.remove (.-classList row) "shui-shortcut-row--pressed")))))
160))))))))
(animate-element! key-el container highlight-row?))
(animate-element! container container highlight-row?)))))))
(rum/defc combo-keys
"Renders combo keys (simultaneous key combinations) with separator."

22
resources/css/shui.css
View File

@@ -283,13 +283,15 @@ div[data-radix-popper-content-wrapper] {
/* Glow effect for combo and separate keys */
/* Combo keys: glow on container (wraps all keys together) */
.shui-shortcut-combo.shui-shortcut-glow {
box-shadow: rgba(255, 255, 255, 0.15) 0px 1px 0px 0px inset, rgba(0, 0, 0, 0.15) 0px -1px 0px 0px inset;
box-shadow: var(--lx-gray-01-alpha, rgba(255, 255, 255, 0.15)) 0px 1px 0px 0px inset,
var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.15)) 0px -1px 0px 0px inset;
border: none;
}
/* Separate keys: glow on individual keys (not container) */
.shui-shortcut-separate.shui-shortcut-glow kbd.shui-shortcut-key {
box-shadow: rgba(255, 255, 255, 0.15) 0px 1px 0px 0px inset, rgba(0, 0, 0, 0.15) 0px -1px 0px 0px inset;
box-shadow: var(--lx-gray-01-alpha, rgba(255, 255, 255, 0.15)) 0px 1px 0px 0px inset,
var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.15)) 0px -1px 0px 0px inset;
border: none;
}
@@ -350,7 +352,7 @@ kbd.shui-shortcut-key,
line-height: 16px;
min-width: fit-content;
transition: transform 140ms ease-out, box-shadow 140ms ease-out;
box-shadow: 0 0 0 rgba(0, 0, 0, 0);
box-shadow: 0 0 0 transparent;
}
/* Keys in separate containers get their own styling */
@@ -383,23 +385,27 @@ kbd.shui-shortcut-key-pressed,
/* Key press animation with glow - preserve glow effect */
/* Combo keys: animate the container */
.shui-shortcut-combo.shui-shortcut-glow.shui-shortcut-key-pressed {
box-shadow: rgba(255, 255, 255, 0.15) 0px 2px 0px 0px inset, rgba(0, 0, 0, 0.15) 0px 0px 0px 0px inset, 0 1px 2px rgba(0, 0, 0, 0.2);
box-shadow: var(--lx-gray-01-alpha, rgba(255, 255, 255, 0.15)) 0px 2px 0px 0px inset,
var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.15)) 0px 0px 0px 0px inset,
0 1px 2px var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.2));
}
/* Separate keys: animate individual keys */
.shui-shortcut-separate.shui-shortcut-glow kbd.shui-shortcut-key-pressed {
box-shadow: rgba(255, 255, 255, 0.15) 0px 2px 0px 0px inset, rgba(0, 0, 0, 0.15) 0px 0px 0px 0px inset, 0 1px 2px rgba(0, 0, 0, 0.2);
box-shadow: var(--lx-gray-01-alpha, rgba(255, 255, 255, 0.15)) 0px 2px 0px 0px inset,
var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.15)) 0px 0px 0px 0px inset,
0 1px 2px var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.2));
}
/* Key press animation without glow */
.shui-shortcut-combo:not(.shui-shortcut-glow) kbd.shui-shortcut-key-pressed,
.shui-shortcut-separate:not(.shui-shortcut-glow) kbd.shui-shortcut-key-pressed {
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.2);
box-shadow: 0 1px 2px var(--lx-gray-12-alpha, rgba(0, 0, 0, 0.2));
}
/* Row highlight animation */
.shui-shortcut-row--pressed {
background-color: rgba(223, 239, 254, 0.1);
background-color: var(--lx-accent-04-alpha, rgba(223, 239, 254, 0.1));
transition: background-color 160ms ease-out;
}
@@ -417,7 +423,7 @@ kbd.shui-shortcut-key-pressed,
.shui-shortcut-row--pressed {
transition: none;
transform: none;
box-shadow: 0 0 0 rgba(0, 0, 0, 0);
box-shadow: 0 0 0 transparent;
}
.shui-shortcut-row--pressed {

71
src/main/frontend/components/shortcut.cljs
View File

@@ -170,10 +170,11 @@
[:div {:class (str "shortcut-input-binding"
(when accumulating? " shortcut-input-binding--pending"))}
(shui/shortcut keystroke)
[:a.shortcut-binding-remove
[:button.shortcut-binding-remove
{:on-click (fn [^js e]
(.stopPropagation e)
(clear!))}
(clear!))
:aria-label "Remove filter"}
(ui/icon "x" {:size 12})]])
;; Placeholder
(when-not has-keystroke?
@@ -184,7 +185,7 @@
[:div.shortcut-toolbar
[:div
(when has-keystroke?
[:a.shortcut-toolbar-action
[:button.shortcut-toolbar-action
{:on-click clear!}
(ui/icon "rotate" {:size 12})
[:span "Clear"]])]
@@ -221,10 +222,11 @@
(set-q! v))}]
(when-not (string/blank? q)
[:a.x
[:button.x
{:on-click (fn []
(set-q! "")
(js/setTimeout #(some-> (rum/deref *search-ref) (.focus)) 50))}
(js/setTimeout #(some-> (rum/deref *search-ref) (.focus)) 50))
:aria-label "Clear search"}
(ui/icon "x" {:size 12})])]
;; keystroke filter button
@@ -253,10 +255,11 @@
[:span.shortcut-keystroke-keys
(ui/icon "keyboard" {:size 14})
(shui/shortcut keystroke)]
[:a.shortcut-keystroke-clear
[:button.shortcut-keystroke-clear
{:on-click (fn [^js e]
(.stopPropagation e)
(set-keystroke! ""))}
(set-keystroke! ""))
:aria-label "Clear keystroke filter"}
(ui/icon "x" {:size 12})]]
[:button.shortcut-keystroke-inactive
{:on-click open-filter!}
@@ -280,14 +283,14 @@
(when (string/blank? q)
[:div.flex.items-center.gap-2
[:a.flex.items-center.icon-link
[:button.flex.items-center.icon-link
{:on-click toggle-categories-fn
:title "Toggle categories pane"}
:aria-label "Toggle categories pane"}
(ui/icon "fold")]
[:a.flex.items-center.icon-link
[:button.flex.items-center.icon-link
{:on-click refresh-shortcuts-list!
:title "Refresh all"}
:aria-label "Refresh all"}
(ui/icon "refresh")]])]]))
(rum/defc shortcut-desc-label
@@ -357,9 +360,10 @@
:when (not (nil? m))]
[:li
{:key (str id')}
[:a.select-none.hover:underline
[:button.select-none.hover:underline
{:on-click (fn [^js e] (open-customize-shortcut-dialog! e id'))
:title (str handler-id)}
:title (str handler-id)
:aria-label (dh/get-shortcut-desc m)}
[:code.inline-block.mr-1.text-xs
(shortcut-utils/decorate-binding k)]
[:span
@@ -647,8 +651,9 @@
[:div.shortcut-input-binding {:key x}
(shui/shortcut x)
(when (#{:idle :accepted :esc-hint :removed :reset} rec-state)
[:a.shortcut-binding-remove
{:on-click (fn [^js e]
[:button.shortcut-binding-remove
{:aria-label "Remove binding"
:on-click (fn [^js e]
(.stopPropagation e)
(let [new-binding (vec (concat (subvec current-binding 0 idx)
(subvec current-binding (inc idx))))
@@ -664,8 +669,9 @@
[:div.shortcut-input-binding.shortcut-input-binding--pending
(shui/shortcut keystroke)
(when (#{:conflict-cross :conflict-same} rec-state)
[:a.shortcut-binding-remove
{:on-click (fn [^js e]
[:button.shortcut-binding-remove
{:aria-label "Remove binding"
:on-click (fn [^js e]
(.stopPropagation e)
(cancel-fn!))}
(ui/icon "x" {:size 12})])])
@@ -676,7 +682,7 @@
;; FEEDBACK BANNER (conditional)
(let [undo-link
(when undo-snapshot
[:a.shortcut-feedback-action
[:button.shortcut-feedback-action
{:on-click (fn []
(execute-undo! undo-snapshot)
(when-let [own (some #(when (= (:action-id %) k) %) (:entries undo-snapshot))]
@@ -743,7 +749,7 @@
;; Reset (only when changed from default)
(when (and (#{:idle :accepted :removed} rec-state)
(not= current-binding binding))
[:a.shortcut-toolbar-action
[:button.shortcut-toolbar-action
{:on-click reset-fn!}
(ui/icon "rotate" {:size 12})
[:span "Reset"]])]
@@ -798,6 +804,17 @@
{:All 0 :Custom 0 :Unset 0 :Disabled 0}
all-bindings)))
(defn- matches-keystroke?
"Check if any of the shortcut's bindings match the recorded keystroke filter."
[binding user-binding keystroke]
(let [binding' (or user-binding binding)
keystroke' (some-> (shortcut-utils/safe-parse-string-binding keystroke) (bean/->clj))]
(when (sequential? binding')
(some #(when-let [s (some-> % (dh/mod-key) (shortcut-utils/safe-parse-string-binding) (bean/->clj))]
(or (= s keystroke')
(and (sequential? s) (sequential? keystroke')
(apply = (map first [s keystroke']))))) binding'))))
(defn- count-visible-shortcuts
"Count shortcuts visible after applying category filter and keystroke filter."
[result-list-map filter-key in-keystroke? keystroke]
@@ -815,13 +832,7 @@
(contains? cats filter-key))
:when (or (not in-keystroke?)
(and (not disabled?) (not unset?)
(let [binding' (or user-binding binding)
keystroke' (some-> (shortcut-utils/safe-parse-string-binding keystroke) (bean/->clj))]
(when (sequential? binding')
(some #(when-let [s (some-> % (dh/mod-key) (shortcut-utils/safe-parse-string-binding) (bean/->clj))]
(or (= s keystroke')
(and (sequential? s) (sequential? keystroke')
(apply = (map first [s keystroke']))))) binding')))))]
(matches-keystroke? binding user-binding keystroke)))]
id)))
count))
@@ -935,13 +946,7 @@
(when (or (not in-keystroke?)
(and (not disabled?)
(not unset?)
(let [binding' (or user-binding binding)
keystroke' (some-> (shortcut-utils/safe-parse-string-binding keystroke) (bean/->clj))]
(when (sequential? binding')
(some #(when-let [s (some-> % (dh/mod-key) (shortcut-utils/safe-parse-string-binding) (bean/->clj))]
(or (= s keystroke')
(and (sequential? s) (sequential? keystroke')
(apply = (map first [s keystroke']))))) binding')))))
(matches-keystroke? binding user-binding keystroke)))
(let [row-action (when (and id (not disabled?))
(fn [^js e]

17
src/main/frontend/components/shortcut.css
View File

@@ -13,6 +13,23 @@
}
}
/* Reset button styles for interactive elements changed from <a> to <button> */
.cp__shortcut-page-x button.shortcut-binding-remove,
.cp__shortcut-page-x button.shortcut-toolbar-action,
.cp__shortcut-page-x button.shortcut-keystroke-clear,
.cp__shortcut-page-x button.icon-link,
.cp__shortcut-page-x button.x,
.shortcut-popover button.shortcut-binding-remove,
.shortcut-popover button.shortcut-toolbar-action,
.shortcut-filter-popover button.shortcut-binding-remove,
.shortcut-filter-popover button.shortcut-toolbar-action,
button.shortcut-feedback-action {
all: unset;
cursor: pointer;
display: inline-flex;
align-items: center;
}
.cp__shortcut-page-x {
@apply relative;