Conversation
…rl+Shift+L) Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/0b106b55-177a-42e8-8c01-a25552f368cb Co-authored-by: emako <24737061+emako@users.noreply.github.com>
…or performance Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/0b106b55-177a-42e8-8c01-a25552f368cb Co-authored-by: emako <24737061+emako@users.noreply.github.com>
…tation Agent-Logs-Url: https://github.com/QL-Win/QuickLook/sessions/0b106b55-177a-42e8-8c01-a25552f368cb Co-authored-by: emako <24737061+emako@users.noreply.github.com>
Ensure the table-of-contents overflow is explicitly set to "auto" after the transition to override any lingering --toc-overflow-y CSS variable, replacing the previous empty string assignment. Also expose toggleToc on window so the host application (e.g. C# via ExecuteScriptAsync) can invoke it programmatically.
Temporarily disable the TOC element's CSS transition while the user is dragging the resize handle so the width updates immediately; restore the transition when the drag ends. This change in md2html.html prevents delayed width updates during TOC resizing for a smoother drag experience.
Reviewer's GuideAdds a configurable keyboard shortcut (default Ctrl+Shift+L) to toggle the Markdown viewer’s Table of Contents visibility with a smooth animated collapse/expand, persists state and width in localStorage, guards resize behavior when collapsed, and wires the shortcut through plugin configuration and documentation. Sequence diagram for Markdown TOC toggle keyboard shortcut handlingsequenceDiagram
actor User
participant MarkdownViewerWindow
participant MarkdownViewerScript
participant localStorage
User->>MarkdownViewerWindow: Press Ctrl+Shift+L or configured shortcut
MarkdownViewerWindow->>MarkdownViewerScript: keydown event
MarkdownViewerScript->>MarkdownViewerScript: matchesShortcut(e, TOC_TOGGLE_SHORTCUT)
alt Shortcut matches and count >= 2
MarkdownViewerScript->>MarkdownViewerScript: toggleToc()
alt TOC currently visible
MarkdownViewerScript->>MarkdownViewerScript: set width 0px, minWidth 0px, overflowY hidden
MarkdownViewerScript->>localStorage: setItem(tocVisible, false)
else TOC currently hidden
MarkdownViewerScript->>localStorage: getItem(tocWidth) or 178px
MarkdownViewerScript->>MarkdownViewerScript: set width to saved width, minWidth 4px
MarkdownViewerScript->>localStorage: setItem(tocVisible, true)
MarkdownViewerScript->>MarkdownViewerScript: setTimeout(restore overflowY auto after TOC_TRANSITION_MS)
end
else Shortcut does not match
MarkdownViewerScript->>MarkdownViewerScript: handle other shortcuts (e.g. RTL/LTR)
end
Sequence diagram for injecting configurable TOC shortcut into Markdown HTMLsequenceDiagram
participant QuickLookHost
participant MarkdownPanel
participant SettingHelper
participant Template as md2html_html_template
participant WebView
QuickLookHost->>MarkdownPanel: GenerateMarkdownHtml(path)
MarkdownPanel->>SettingHelper: Get(ToggleTocKey, Ctrl+Shift+L, QuickLook.Plugin.MarkdownViewer)
SettingHelper-->>MarkdownPanel: tocShortcut string
MarkdownPanel->>Template: Replace {{content}}, {{rtl}}, {{tocShortcut}}
Template-->>MarkdownPanel: Final HTML
MarkdownPanel-->>QuickLookHost: HTML string
QuickLookHost->>WebView: Load HTML
WebView->>WebView: TOC_TOGGLE_SHORTCUT = injected tocShortcut
Updated class diagram for MarkdownPanel TOC shortcut configurationclassDiagram
class MarkdownPanel {
+GenerateMarkdownHtml(path string) string
}
class SettingHelper {
+Get(key string, defaultValue string, section string) string
}
MarkdownPanel ..> SettingHelper : uses for ToggleTocKey
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
tocShortcutvalue is injected directly into the JS string literal (const TOC_TOGGLE_SHORTCUT = "{{tocShortcut}}";) without escaping, so a config value containing quotes or backslashes could break the script or alter behavior; consider HTML/JS-escaping the value before template substitution or passing it via a data attribute instead. - The
matchesShortcutimplementation splits on"+"which makes it impossible to express keys whoseKeyboardEvent.keyis literally"+"(and similar edge cases), so you may want to either document this limitation or adjust the parsing to support such keys (e.g., using named tokens likePlusor a different separator).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `tocShortcut` value is injected directly into the JS string literal (`const TOC_TOGGLE_SHORTCUT = "{{tocShortcut}}";`) without escaping, so a config value containing quotes or backslashes could break the script or alter behavior; consider HTML/JS-escaping the value before template substitution or passing it via a data attribute instead.
- The `matchesShortcut` implementation splits on `"+"` which makes it impossible to express keys whose `KeyboardEvent.key` is literally `"+"` (and similar edge cases), so you may want to either document this limitation or adjust the parsing to support such keys (e.g., using named tokens like `Plus` or a different separator).
## Individual Comments
### Comment 1
<location path="QuickLook.Plugin/QuickLook.Plugin.MarkdownViewer/MarkdownPanel.cs" line_range="97-100" />
<code_context>
isRtl = true;
}
+ var tocShortcut = SettingHelper.Get("ToggleTocKey", "Ctrl+Shift+L", "QuickLook.Plugin.MarkdownViewer");
+
var html = template.Replace("{{content}}", content)
- .Replace("{{rtl}}", isRtl ? "rtl" : "ltr");
+ .Replace("{{rtl}}", isRtl ? "rtl" : "ltr")
+ .Replace("{{tocShortcut}}", tocShortcut);
</code_context>
<issue_to_address>
**🚨 issue (security):** Safely serialize `tocShortcut` for insertion into the HTML/JS template.
`tocShortcut` is currently injected directly into a JS string literal in the template. If a user sets it to a value containing quotes or special characters, it can break the HTML/JS or allow script injection. Please ensure it’s properly escaped before `Replace("{{tocShortcut}}", ...)`, for example by serializing it with `JsonSerializer.Serialize(tocShortcut)` and then emitting `const TOC_TOGGLE_SHORTCUT = ${jsShortcut};` in the template.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| var tocShortcut = SettingHelper.Get("ToggleTocKey", "Ctrl+Shift+L", "QuickLook.Plugin.MarkdownViewer"); | ||
|
|
||
| var html = template.Replace("{{content}}", content) | ||
| .Replace("{{rtl}}", isRtl ? "rtl" : "ltr"); | ||
| .Replace("{{rtl}}", isRtl ? "rtl" : "ltr") |
There was a problem hiding this comment.
🚨 issue (security): Safely serialize tocShortcut for insertion into the HTML/JS template.
tocShortcut is currently injected directly into a JS string literal in the template. If a user sets it to a value containing quotes or special characters, it can break the HTML/JS or allow script injection. Please ensure it’s properly escaped before Replace("{{tocShortcut}}", ...), for example by serializing it with JsonSerializer.Serialize(tocShortcut) and then emitting const TOC_TOGGLE_SHORTCUT = ${jsShortcut}; in the template.
There was a problem hiding this comment.
Pull request overview
Adds a configurable keyboard shortcut to toggle the Markdown viewer’s Table of Contents (TOC) panel, including a persisted collapsed/expanded state and a width transition animation, with documentation for the new plugin config option.
Changes:
- Update
md2html.htmlto support no-flicker initial TOC state, animated TOC collapse/expand, and a shortcut parser/handler. - Update
MarkdownPanel.csto readToggleTocKeyfromQuickLook.Plugin.MarkdownViewer.configand inject it into the HTML template. - Document the new
ToggleTocKeyoption inOPTIONS.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| QuickLook.Plugin/QuickLook.Plugin.MarkdownViewer/Resources/md2html.html | Adds TOC visibility persistence, slide animation, shortcut matching, and resize guarding when collapsed. |
| QuickLook.Plugin/QuickLook.Plugin.MarkdownViewer/MarkdownPanel.cs | Reads ToggleTocKey from plugin config and injects it into the HTML template. |
| OPTIONS.md | Documents the new MarkdownViewer config option ToggleTocKey. |
| // Helper: check if a KeyboardEvent matches a shortcut string like "Ctrl+Shift+L" | ||
| // Supported modifiers: Ctrl, Shift, Alt. Key name is case-insensitive and matches e.key. | ||
| function matchesShortcut(e, shortcut) { | ||
| if (!shortcut) return false; | ||
| var parts = shortcut.toLowerCase().split("+").map(function (p) { return p.trim(); }).filter(Boolean); | ||
| if (parts.length === 0) return false; | ||
| var key = parts[parts.length - 1]; | ||
| return e.key.toLowerCase() === key | ||
| && e.ctrlKey === parts.includes("ctrl") | ||
| && e.shiftKey === parts.includes("shift") | ||
| && e.altKey === parts.includes("alt"); | ||
| } | ||
|
|
||
| const TOC_TOGGLE_SHORTCUT = "{{tocShortcut}}"; | ||
|
|
| var tocShortcut = SettingHelper.Get("ToggleTocKey", "Ctrl+Shift+L", "QuickLook.Plugin.MarkdownViewer"); | ||
|
|
||
| var html = template.Replace("{{content}}", content) | ||
| .Replace("{{rtl}}", isRtl ? "rtl" : "ltr"); | ||
| .Replace("{{rtl}}", isRtl ? "rtl" : "ltr") | ||
| .Replace("{{tocShortcut}}", tocShortcut); |
No keyboard shortcut existed to toggle the Markdown viewer's Table of Contents panel. This adds
Ctrl+Shift+L(matching Typora) as the default, fully configurable via the plugin config file.Changes
md2html.htmltocVisiblefromlocalStoragebefore paint and sets--toc-width,--toc-min-width,--toc-overflow-yCSS variables immediately, so a previously-collapsed TOC doesn't flicker on load#tocgainstransition: width 0.3s ease;min-widthandoverflow-ynow use CSS variables so they can be overridden at paint timetoggleToc(): Collapses/expands width with a 300ms slide animation (TOC_TRANSITION_MSconstant keeps CSS and JS duration in sync); state persisted inlocalStoragematchesShortcut(e, shortcut): Parses shortcut strings like"Ctrl+Shift+L"— supportsCtrl,Shift,Altmodifiers, case-insensitive key name, filters empty/malformed partstocIsVisibleboolean (vs. repeatedlocalStorage.getItem) gates the resize-handle hit-test, preventing accidental resize interaction on a collapsed TOCMarkdownPanel.csReads
ToggleTocKeyfromQuickLook.Plugin.MarkdownViewer.configviaSettingHelper.Get(defaultCtrl+Shift+L) and injects it as{{tocShortcut}}into the HTML template.OPTIONS.mdNew
QuickLook.Plugin.MarkdownViewer.config optionssection documentingToggleTocKey.Configuration
Supported modifiers:
Ctrl,Shift,Alt. Key name is matched case-insensitively againstKeyboardEvent.key(e.g.L,F1,Escape).Summary by Sourcery
Add a configurable keyboard shortcut to toggle the Markdown viewer’s Table of Contents panel with smooth animated show/hide behavior and persist its state across sessions.
New Features:
Enhancements:
Documentation: