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

Address pr-review findings: accessibility, dead code, and consistency fixes

Browse Source 
- Add ARIA live regions to feedback banners (role=alert/status)
- Add role=dialog and aria-label to customize popover
- Add aria-pressed to filter pill buttons
- Remove disabled rows from tab order
- Return focus to trigger on popover close
- Add shortcut-badge-in to prefers-reduced-motion
- Remove dead shortcut-conflicts-display component
- Replace js/console.warn with lambdaisland.glogi
- Merge duplicate cond branches in key handler
- Add missing :shortcut-id to left sidebar

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
scheinriese
2026-03-11 12:16:53 +01:00
committed by Tienson Qin
parent f6668aef37
commit ed0d6f0142
4 changed files with 85 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/main/frontend/components/left_sidebar.cljs
View File

@@ -123,7 +123,8 @@
(when shortcut
[:span.ml-1
(ui/render-keyboard-shortcut
(ui/keyboard-shortcut-from-config shortcut {:pick-first? true}))])
(ui/keyboard-shortcut-from-config shortcut {:pick-first? true})
:shortcut-id shortcut)])
more]])
(rum/defc sidebar-graphs

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

@@ -312,6 +312,7 @@
[:button.shortcut-filter-pill
{:key (name k)
:class (when active? "shortcut-filter-pill--active")
:aria-pressed (str active?)
:on-click #(set-filter-key! (when-not (or (= k :All) (= filter-key k)) k))}
[:span.shortcut-filter-pill-title title]
[:span.shortcut-filter-pill-count (str " \u00B7 " cnt)]])]
@@ -363,7 +364,8 @@
{:id popup-id
:force-popover? true
:align "start"
:on-after-hide #(reset! *active-shortcut-id nil)
:on-after-hide #(do (reset! *active-shortcut-id nil)
(when anchor-el (.focus anchor-el)))
:content-props
{:class "p-0 w-auto"
:collision-padding 12
@@ -376,36 +378,6 @@
(.preventDefault e)
false)))}}))))
(rum/defc shortcut-conflicts-display
[_k conflicts-map]
[:div.cp__shortcut-conflicts-list-wrap
(for [[g ks] conflicts-map]
^{:key (str g)}
[:section.relative
[:h2 (ui/icon "alert-triangle" {:size 15})
[:span (t :keymap/conflicts-for-label)]
[:code (shortcut-utils/decorate-binding g)]]
[:ul
(for [v (vals ks)
:let [k (first v)
vs (second v)]
[id' handler-id] vs
:let [m (dh/shortcut-item id')]
:when (not (nil? m))]
[:li
{:key (str id')}
[:button.select-none.hover:underline
{:on-click (fn [^js e] (open-customize-shortcut-dialog! e 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
(dh/get-shortcut-desc m)
(ui/icon "external-link" {:size 18})]
[:code [:small (str id')]]]])]])])
(defn- execute-undo!
"Restore previous bindings for all affected actions."
[snapshot]
@@ -649,14 +621,8 @@
(= state :conflict-cross)
nil
;; Conflict-same / esc-hint + key => start new recording
(#{:conflict-same :esc-hint} state)
(when-let [kn (shortcut/keyname e)]
(set-rec-state! :recording)
(set-keystroke! (util/trim-safe kn)))
;; Idle / accepted / removed / reset / dismissing + key => start recording
(#{:idle :accepted :removed :reset :dismissing} state)
;; Any non-recording, non-conflict-cross state + key => start new recording
(#{:conflict-same :esc-hint :idle :accepted :removed :reset :dismissing} state)
(when-let [kn (shortcut/keyname e)]
(set-rec-state! :recording)
(set-keystroke! (util/trim-safe kn)))
@@ -675,7 +641,9 @@
;; === V3 LAYOUT ===
[:div.shortcut-popover
{:tab-index -1
:ref *ref-el}
:ref *ref-el
:role "dialog"
:aria-label action-name}
;; TITLE
[:div.shortcut-popover-title action-name]
@@ -717,79 +685,81 @@
(when (#{:idle :recording :accepted :removed :reset} render-state)
[:span.shortcut-input-placeholder (t :keymap/press-a-shortcut)])]
;; FEEDBACK BANNER (conditional)
(let [undo-link
(when undo-snapshot
[:button.shortcut-feedback-action
{:on-click (fn []
(execute-undo! undo-snapshot)
(when-let [own (some #(when (= (:action-id %) k) %) (:entries undo-snapshot))]
(set-current-binding! (:previous-binding own)))
(set-undo-snapshot! nil)
(set-rec-state! :idle))}
(t :keymap/undo)])]
(case rec-state
:conflict-cross
[:div.shortcut-feedback.shortcut-feedback--error
[:span (t :keymap/used-by)
[:span.shortcut-feedback-name (conflict-action-names key-conflicts)]]
(ui/tooltip
(shui/button {:variant :destructive
:size :xs
:on-click override-fn!}
(t :keymap/reassign))
(t :keymap/reassign-tooltip))]
;; FEEDBACK BANNER (conditional) — wrapped in live region for screen readers
[:div {:role (if (#{:conflict-cross :conflict-same} rec-state) "alert" "status")
:aria-live (if (#{:conflict-cross :conflict-same} rec-state) "assertive" "polite")}
(let [undo-link
(when undo-snapshot
[:button.shortcut-feedback-action
{:on-click (fn []
(execute-undo! undo-snapshot)
(when-let [own (some #(when (= (:action-id %) k) %) (:entries undo-snapshot))]
(set-current-binding! (:previous-binding own)))
(set-undo-snapshot! nil)
(set-rec-state! :idle))}
(t :keymap/undo)])]
(case rec-state
:conflict-cross
[:div.shortcut-feedback.shortcut-feedback--error
[:span (t :keymap/used-by)
[:span.shortcut-feedback-name (conflict-action-names key-conflicts)]]
(ui/tooltip
(shui/button {:variant :destructive
:size :xs
:on-click override-fn!}
(t :keymap/reassign))
(t :keymap/reassign-tooltip))]
:conflict-same
[:div.shortcut-feedback.shortcut-feedback--error
[:span (t :keymap/already-bound)]]
:conflict-same
[:div.shortcut-feedback.shortcut-feedback--error
[:span (t :keymap/already-bound)]]
:accepted
(cond
(:cross-context? accepted-info)
[:div.shortcut-feedback.shortcut-feedback--warning
[:span (t :keymap/also-used-for)
[:span.shortcut-feedback-name
(:cross-action-name accepted-info)]
(when-let [ctx (:cross-context-label accepted-info)]
(str (t :keymap/in-context) ctx))]]
:accepted
(cond
(:cross-context? accepted-info)
[:div.shortcut-feedback.shortcut-feedback--warning
[:span (t :keymap/also-used-for)
[:span.shortcut-feedback-name
(:cross-action-name accepted-info)]
(when-let [ctx (:cross-context-label accepted-info)]
(str (t :keymap/in-context) ctx))]]
(:from accepted-info)
[:div.shortcut-feedback.shortcut-feedback--success
[:span (t :keymap/reassigned-from)
[:span.shortcut-feedback-name (:from accepted-info)]]
undo-link]
(:from accepted-info)
[:div.shortcut-feedback.shortcut-feedback--success
[:span (t :keymap/reassigned-from)
[:span.shortcut-feedback-name (:from accepted-info)]]
undo-link]
:else
[:div.shortcut-feedback.shortcut-feedback--success
[:span (t :keymap/shortcut-added)]])
:else
[:div.shortcut-feedback.shortcut-feedback--success
[:span (t :keymap/shortcut-added)]])
:removed
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/shortcut-removed)]
undo-link]
:removed
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/shortcut-removed)]
undo-link]
:reset
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/reset-to-default)]
undo-link]
:reset
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/reset-to-default)]
undo-link]
:esc-hint
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/esc-is-reserved)]]
:esc-hint
[:div.shortcut-feedback.shortcut-feedback--muted
[:span (t :keymap/esc-is-reserved)]]
:dismissing
(let [prev (rum/deref *prev-rec-state)
variant (case prev
(:conflict-cross :conflict-same) "shortcut-feedback--error"
:accepted (if (:cross-context? accepted-info)
"shortcut-feedback--warning"
"shortcut-feedback--success")
"shortcut-feedback--muted")]
[:div {:class (str "shortcut-feedback " variant " is-dismissing")
:on-animation-end #(set-rec-state! :idle)}])
:dismissing
(let [prev (rum/deref *prev-rec-state)
variant (case prev
(:conflict-cross :conflict-same) "shortcut-feedback--error"
:accepted (if (:cross-context? accepted-info)
"shortcut-feedback--warning"
"shortcut-feedback--success")
"shortcut-feedback--muted")]
[:div {:class (str "shortcut-feedback " variant " is-dismissing")
:on-animation-end #(set-rec-state! :idle)}])
nil))
nil))]
;; SEPARATOR + TOOLBAR
(shui/separator)
@@ -1049,7 +1019,7 @@
[:li.shortcut-row.flex.items-start.justify-between.text-sm
{:key (str id)
:class (when (= active-id id) "active")
:tab-index (when id 0)
:tab-index (when (and id (not disabled?)) 0)
:role (when id "button")
:aria-disabled (when disabled? "true")
:on-click row-action

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

@@ -493,6 +493,11 @@ button.shortcut-feedback-action {
}
@media (prefers-reduced-motion: reduce) {
@keyframes shortcut-badge-in {
from { opacity: 0; }
to { opacity: 1; }
}
@keyframes shortcut-fade-in {
from { opacity: 0; }
to { opacity: 1; }

5
src/main/frontend/modules/shortcut/utils.cljs
View File

@@ -1,6 +1,7 @@
(ns frontend.modules.shortcut.utils
(:require [clojure.string :as string]
[frontend.util :as util])
[frontend.util :as util]
[lambdaisland.glogi :as log])
(:import [goog.ui KeyboardShortcutHandler]))
(def ^:private modifier-order
@@ -39,7 +40,7 @@
(try
(KeyboardShortcutHandler/parseStringShortcut binding)
(catch js/Error e
(js/console.warn "[shortcuts] parse key error: " e) binding)))
(log/warn :shortcut/parse-key-error {:error e :binding binding}) binding)))
(defn mod-key [binding]
(string/replace binding #"(?i)mod"