Conversation
…uttons, and editor focus Co-authored-by: cyanzhong <[email protected]>
Co-authored-by: cyanzhong <[email protected]>
|
@copilot the current implementation is extremely rough.
Focus on these first, I am sure there are more problems. |
…ability Co-authored-by: cyanzhong <[email protected]>
All four issues addressed in a19887f:
|
|
@copilot three big problems:
Please work on these details. |
…rders and spacing Co-authored-by: cyanzhong <[email protected]>
All four addressed in 60b6fdf:
|
|
@copilot for
For other details:
|
…pacious layout, total count, card shadows Co-authored-by: cyanzhong <[email protected]>
All six points addressed in f3ee852:
|
|
@copilot Looks getting better. More issues:
|
…ations Co-authored-by: cyanzhong <[email protected]>
|
@copilot Can we make the sidebar a bit narrower? Just a bit. Also, I think currently navigating between cateories is hard if we have lots of suggestions. What do you think? Make the section collapsable or add a filter? Or do nothing since we prefer clean UI? Think as senior designer. |
All three addressed in cc62354:
|
Fixed in b841b91 — accepting a suggestion now animates the card out with a smooth opacity fade + height collapse transition (~250ms). After the animation completes, section counts and total count update. Empty sections are removed, and an "No problems found." message appears when all issues are fixed. The animation uses CSS transitions: opacity fades first (150ms), then max-height/margin/padding collapse (200ms with 50ms stagger), creating a natural "shrink away" effect. A guard prevents double-clicks during the animation. |
|
@copilot What's the reasoning we don't have "Ignore" in sidebar? Should we also just have it? |
Co-authored-by: cyanzhong <[email protected]>
|
@copilot Now I am worried about your skills. Since the card in popup and sidebar are almost identical (with only a few UI differences like hover), shouldn't we be reusing components? Don't repeat yourself. Think about the architecture, as a senior engineer. |
Yes, it should be there — the popup tooltip cards already had it and the sidebar was missing parity. Fixed in 7dae1fe — each sidebar card now has an "Ignore" button that removes the individual diagnostic (and adds to dictionary if configured), with the same animated card dismissal. Styled to match the popup tooltip's |
…anel) Co-authored-by: cyanzhong <[email protected]>
Extracted all shared card-building logic into
Tooltip and panel only retain their own container-specific CSS with size overrides (e.g. |
|
@copilot It looks like we should hide "Ignore" in sidebar mode, because we have fixed width: I am seeing strange things like this:
Please remove "Ignore" in sidebar, and make sure we don't wrap lines like this. Also, you know what kind of code convention we are looking for. Do self-review to wrap this up. |
Co-authored-by: cyanzhong <[email protected]>
Co-authored-by: cyanzhong <[email protected]>
Fixed in ddcf45e:
All 35 tests pass, lint clean, no CodeQL alerts. |
There was a problem hiding this comment.
Pull request overview
Adds a dedicated right-side “Review Problems” pane to review lint suggestions in one place, and refactors tooltip/pane card rendering into a shared card module to reduce duplication.
Changes:
- Introduces
panelExtension(sidebar pane) and a new “Review Problems” menu action that lints then opens the pane. - Extracts shared card DOM/CSS + diagnostic helpers into
src/card.tsand refactorstooltip.tsto use it. - Improves diagnostic robustness by mapping
Diagnostic.from/tothrough document changes; updates/extends CSS-oriented tests accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/panel.ts | New ViewPlugin-based sidebar pane rendering grouped diagnostics and providing UI interactions/CSS. |
| src/card.ts | New shared card builder/helpers + shared base CSS injection for tooltip/pane card content. |
| src/tooltip.ts | Refactor tooltip card to reuse shared card helpers/CSS; exports setClickTooltip. |
| src/menu.ts | Adds “Review Problems” item; runs lint then opens pane and dismisses click tooltip. |
| src/extension.ts | Registers panelExtension in the main extension array. |
| src/decoration.ts | Maps stored diagnostics’ from/to through tr.changes and filters collapsed ranges. |
| src/styling.ts | Exports kindColors / kindColorsDark and brightens dark-mode palette/fallback. |
| tests/panel.test.ts | New tests for pane CSS and shared cardContentCSS output. |
| tests/menu.test.ts | Updates menu structure assertions for the new menu item/separators. |
| tests/styling.test.ts | Updates dark-mode fallback underline color assertion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
af322e4 to
5904151
Compare
|
@copilot Final self-review. |
…Diagnostic Co-authored-by: cyanzhong <[email protected]>
Self-review complete in 362b3fa. Three issues fixed:
All 35 tests pass, lint clean, no CodeQL alerts. |






Introduces a right-side task pane for reviewing all lint suggestions in one place, grouped by category.
New:
src/card.tsShared card-building module extracted from duplicated logic in
tooltip.tsandpanel.ts. Both the popup tooltip cards and the sidebar pane cards now reuse the same helpers and base CSS, eliminating repeated code for message rendering, action buttons, ignore buttons, accent coloring, and diagnostic lookup/removal:setAccentColor(el, lintKind)— sets--harper-kind-color/--harper-kind-color-darkCSS custom properties on a DOM element using the sharedkindColors/kindColorsDarkmapsfindDiagnostic(view, diag)— looks up the current (position-mapped) diagnostic matching byfrom/to/lintKindignoreDiagnostic(view, diag, extraEffects)— removes a diagnostic from state, optionally adds the word to the user dictionary, accepts extra state effects (e.g. closing a tooltip)buildCardContent(container, view, diagnostic, callbacks)— builds the inner card content (message HTML + action buttons row with suggestion buttons and optional "Ignore" button), with configurableguard,onApply,onIgnorecallbacks andshowIgnoreflag for context-specific behavior (e.g. dismiss animation in the panel, tooltip closure in the popup; sidebar hides "Ignore" to fit within fixed width)cardContentCSS()— returns shared base CSS for.harper-msg,.harper-msg code(accentcolor-mix()pattern),.harper-actions,.harper-btn,.harper-ignore, plus dark mode overridesinjectCardCSS()— injects the shared CSS once via a#harper-card-base-styleselementNew:
src/panel.tsCustom sidebar activated via
togglePanelEffect, implemented as aViewPluginthat manages a.harper-paneDOM element inside.cm-editor. The pane is absolutely positioned on the right side of the editor, withmargin-rightapplied directly toscrollDOMon open/close to reliably prevent content overlap. The pane slides in/out with atransform: translateXanimation (0.2s ease-out), and the editor margin animates in sync.Diagnostics are grouped by
lintKind, each section headed by a colored badge with count. Items show the flagged word, message HTML, and action buttons inside bordered cards with subtle shadows.<code>elements inside messages are decorated with the category's accent color usingcolor-mix()and CSS custom properties. Clicking an item selects the range in the editor and scrolls to center it in the viewport usingEditorView.scrollIntoViewwithy: 'center'.overflow-y: auto+min-height: 0for proper flex scrolling), grouped by kind1px solidborders, subtlebox-shadow, and8pxspacing for clear visual separation, with enhanced hover/active statesbuildCardContent()fromcard.tswithshowIgnore: false— sidebar cards show only suggestion action buttons (no per-card "Ignore") to prevent button wrapping within the fixed 290px width; action row usesflex-wrap: nowrap+overflow: hiddenfor safety<code>in messages — tinted background and text color matching the category via--harper-kind-colorCSS custom properties, applied consistently in both the sidebar pane and popup tooltip cards through the sharedcardContentCSS()translateXtransition, editor margin animates in sync, close waits fortransitionendbefore DOM removal; RAF stored and cancelled on close/destroy to prevent stale DOM mutations; inlinescrollDOM.style.transitioncleared after close animation completesopacity 0+max-heightcollapse over0.25s ease-out) before removing the DOM element and updating totals; if the last card in a section is dismissed, the section heading is removed too; if all cards are dismissed, the pane auto-closessetDiagnosticsEffectchangesMenu
New "Review Problems" item after "Proofread Now", followed by a separator. "Review Problems" runs proofreading first (lints the document), then opens the pane with fresh results. Any currently presented popup tooltip card is automatically dismissed when the pane opens. A doc snapshot guard bails out if the document changed during async linting, matching the pattern in
lintScheduler:Other changes
src/decoration.ts—diagnosticsFieldnow maps diagnosticfrom/topositions throughtr.changeson document edits (not just decorations), so pane card clicks and action buttons use correct ranges even after edits; stale diagnostics whose ranges collapse are filtered outsrc/styling.ts— exportedkindColors/kindColorsDarkfor reuse by both the pane and tooltip; brightened allkindColorsDarkvalues ~15-20% and raisedfallbackDarkfrom#9CA3AFto#B8C0CCfor better dark mode visibilitysrc/tooltip.ts— exportedsetClickTooltipfor reuse by menu actions; refactored to use sharedbuildCardContent(),setAccentColor(),findDiagnostic(), andignoreDiagnostic()fromcard.ts; popup cards show the "Ignore" button (unlike the sidebar) since popup width is unconstrained; dark mode popup card buttons use the same barely perceptible cool tint as the sidebarsrc/menu.ts—reviewProblemsdispatchessetClickTooltip.of(null)to dismiss any open popup tooltip card before opening the sidebar pane; captures a doc snapshot before asynclint()and bails if the document changed during lintingsrc/extension.ts— registeredpanelExtensionin the extension arrayOriginal prompt
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.