Skip to content

Add "Review Problems" pane#30

Merged
cyanzhong merged 30 commits intomainfrom
copilot/add-dedicated-proofreading-pane
Mar 15, 2026
Merged

Add "Review Problems" pane#30
cyanzhong merged 30 commits intomainfrom
copilot/add-dedicated-proofreading-pane

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

Introduces a right-side task pane for reviewing all lint suggestions in one place, grouped by category.

New: src/card.ts

Shared card-building module extracted from duplicated logic in tooltip.ts and panel.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-dark CSS custom properties on a DOM element using the shared kindColors/kindColorsDark maps
  • findDiagnostic(view, diag) — looks up the current (position-mapped) diagnostic matching by from/to/lintKind
  • ignoreDiagnostic(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 configurable guard, onApply, onIgnore callbacks and showIgnore flag 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 (accent color-mix() pattern), .harper-actions, .harper-btn, .harper-ignore, plus dark mode overrides
  • injectCardCSS() — injects the shared CSS once via a #harper-card-base-styles element

New: src/panel.ts

Custom sidebar activated via togglePanelEffect, implemented as a ViewPlugin that manages a .harper-pane DOM element inside .cm-editor. The pane is absolutely positioned on the right side of the editor, with margin-right applied directly to scrollDOM on open/close to reliably prevent content overlap. The pane slides in/out with a transform: translateX animation (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 using color-mix() and CSS custom properties. Clicking an item selects the range in the editor and scrolls to center it in the viewport using EditorView.scrollIntoView with y: 'center'.

  • Header: compact title bar with total problem count, "Ignore All" button (clears diagnostics and closes the pane), close button
  • Body: scrollable list (overflow-y: auto + min-height: 0 for proper flex scrolling), grouped by kind
  • Section headings display category badge (12px font, 3px 8px padding) with per-section count
  • Cards have 1px solid borders, subtle box-shadow, and 8px spacing for clear visual separation, with enhanced hover/active states
  • Each card uses shared buildCardContent() from card.ts with showIgnore: false — sidebar cards show only suggestion action buttons (no per-card "Ignore") to prevent button wrapping within the fixed 290px width; action row uses flex-wrap: nowrap + overflow: hidden for safety
  • Accent-colored <code> in messages — tinted background and text color matching the category via --harper-kind-color CSS custom properties, applied consistently in both the sidebar pane and popup tooltip cards through the shared cardContentCSS()
  • Animated show/hide — pane slides from right with translateX transition, editor margin animates in sync, close waits for transitionend before DOM removal; RAF stored and cancelled on close/destroy to prevent stale DOM mutations; inline scrollDOM.style.transition cleared after close animation completes
  • Graceful card dismiss animation — accepting a suggestion fades out the card (opacity 0 + max-height collapse over 0.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-closes
  • Compact 290px width for a clean, non-intrusive sidebar
  • Spacious layout with generous margins and paddings throughout
  • Auto-rerenders on setDiagnosticsEffect changes
  • Full light/dark mode support

Menu

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:

{
  title: 'Review Problems',
  action: reviewProblems,
},
{ separator: true },

Other changes

  • src/decoration.tsdiagnosticsField now maps diagnostic from/to positions through tr.changes on 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 out
  • src/styling.ts — exported kindColors / kindColorsDark for reuse by both the pane and tooltip; brightened all kindColorsDark values ~15-20% and raised fallbackDark from #9CA3AF to #B8C0CC for better dark mode visibility
  • src/tooltip.ts — exported setClickTooltip for reuse by menu actions; refactored to use shared buildCardContent(), setAccentColor(), findDiagnostic(), and ignoreDiagnostic() from card.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 sidebar
  • src/menu.tsreviewProblems dispatches setClickTooltip.of(null) to dismiss any open popup tooltip card before opening the sidebar pane; captures a doc snapshot before async lint() and bails if the document changed during linting
  • src/extension.ts — registered panelExtension in the extension array
  • Tests — updated menu structure assertions (6→8 children), added pane CSS tests (card border/shadow, badge sizing, section heading style, total count, shared card CSS injection)
Original prompt

This section details on the original issue you should resolve

<issue_title>Dedicated proofreading pane</issue_title>
<issue_description>## Goals

The extension is close to the intended experience. It already flags writing issues inline, and provides menu items to trigger proofreading or dismiss flagged words.

The next step is to introduce a dedicated pane that allows users to review all lint suggestions in one place.

Proposal

Add a new menu item, "Review Problems", which opens a task pane on the right side of the editor. This pane presents all lint suggestions as a structured list.

The pane should include:

  • A close button to dismiss the pane.
  • An "Ignore All" action that performs the same operation as the existing menu command.

Suggestions are grouped by lint category, with each category using its associated color. Within each section, all relevant issues are listed and presented with clear details. The visual style should reuse the existing suggestion card design, adapted to appear within a scrollable list.

Selecting an item in the pane should focus the corresponding issue in the editor. The flagged word should become selected, and the editor should ensure the selection is visible to the user.

Requirements

The interface must remain visually consistent with the current design language and interaction patterns.

The pane should feel polished and thoughtfully crafted. Interactions should be smooth and intuitive, with attention to small details such as cursor behavior, hover states, click feedback, and transitions.

The presentation should support rich detail while remaining easy to scan and navigate.</issue_description>

Comments on the Issue (you are @copilot in this section)


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Add dedicated proofreading pane for lint suggestions Add dedicated Review Problems pane Mar 15, 2026
Copilot AI requested a review from cyanzhong March 15, 2026 02:07
@cyanzhong
Copy link
Contributor

@copilot the current implementation is extremely rough.

  1. Add a separator below "Review Problems"
  2. Don't use Panel from codemirror, it opens from the bottom, like a bottom sheet.
  3. Please add a "task pane" / "sidebar" like UI to the right.
  4. The problems container isn't scrollable, wow what a mistake!

Focus on these first, I am sure there are more problems.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot the current implementation is extremely rough.

  1. Add a separator below "Review Problems"
    ...

All four issues addressed in a19887f:

  1. Separator added below "Review Problems" in the menu
  2. Removed CodeMirror showPanel entirely — no longer uses the bottom sheet approach
  3. Pane is now a custom right-side sidebar, absolutely positioned inside .cm-editor with margin-right on .cm-scroller to avoid overlap
  4. Body uses overflow-y: auto + min-height: 0 for proper flex-based scrolling

Pane preview

@cyanzhong
Copy link
Contributor

@copilot three big problems:

  1. The right pane currently covers the editor, the hierarchy is wrong, it shouldn't hide text.
  2. The category accent color should be applied to the section header, not every card
  3. We should add a border to the card
  4. We should add spacing between cards

Please work on these details.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot three big problems:

  1. The right pane currently covers the editor, the hierarchy is wrong, it shouldn't hide text.
    ...

All four addressed in 60b6fdf:

  1. Pane no longer covers editor text — switched from baseTheme CSS to directly setting scrollDOM.style.marginRight when opening/closing, and lowered z-index from 5→1.
  2. Accent color moved to section headers — colored left border now on .harper-pane-section-heading, removed from individual cards.
  3. Cards have bordersborder: 1px solid rgba(0,0,0,0.1) with hover darkening; dark mode uses rgba(255,255,255,0.1).
  4. Spacing between cards — margin increased from 2px to 6px, section margin from 4px to 8px.

@cyanzhong
Copy link
Contributor

@copilot for harper-pane-section-heading:

  1. Remove the vertical line (border-left)
  2. Make the harper-badge bigger

For other details:

  1. Clicking on card to select text should ensure the focus is centered, EditorView.scrollIntoView has a strategy for that
  2. Most margins/paddings used are currently too tight, revamp as a senior design lead
  3. Let's add a total count right after the "Problems" title
  4. Should cards have a shadow? Think as a design lead

…pacious layout, total count, card shadows

Co-authored-by: cyanzhong <[email protected]>
Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot for harper-pane-section-heading:

  1. Remove the vertical line (border-left)
    ...

All six points addressed in f3ee852:

  1. Removed border-left from .harper-pane-section-heading — clean flat heading style now
  2. Bigger badges — 12px font, 3px 8px padding, 6px border-radius
  3. Centered scroll — card click now uses EditorView.scrollIntoView(pos, { y: 'center' }) instead of scrollIntoView: true
  4. Spacious layout — header padding 14px 16px, card margin 8px 12px / padding 10px 12px, section gap 12px, action gap 6px/8px margin-top, body padding 8px 0 16px
  5. Total count — shows next to "Problems" title (e.g. "Problems 8")
  6. Card shadows — subtle box-shadow: 0 1px 3px rgba(0,0,0,0.04) with elevated hover state 0 2px 6px rgba(0,0,0,0.06), including dark mode variants

@cyanzhong
Copy link
Contributor

cyanzhong commented Mar 15, 2026

@copilot Looks getting better.

More issues:

  1. Can we use the category accent color to decorate <code> inside `harper-pane-msg?
  2. The Problems header looks a bit too tall
  3. Can we have show/hide animations?

@cyanzhong
Copy link
Contributor

@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.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot Looks getting better.

More issues:
...

All three addressed in cc62354:

  1. Accent-colored <code> — uses color-mix(in srgb, var(--harper-kind-color) 10%, transparent) for tinted background and var(--harper-kind-color) for text color, so each category's <code> matches its section color. Dark mode uses --harper-kind-color-dark.
  2. Compact header — padding reduced from 14px 16px to 10px 14px.
  3. Slide animations — pane slides in/out with transform: translateX(100%)translateX(0) + transition: 0.2s ease-out. The editor margin animates in sync. Close waits for transitionend before removing the DOM.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot found one more issue, accepting suggestion from the sidebar would lead to a sudden card dismiss, how can we do that gracefully? animations?

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.

@cyanzhong
Copy link
Contributor

@copilot What's the reasoning we don't have "Ignore" in sidebar? Should we also just have it?

@cyanzhong
Copy link
Contributor

@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.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot What's the reasoning we don't have "Ignore" in sidebar? Should we also just have it?

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 .harper-ignore pattern: transparent background, lighter border, pushed to the right via margin-left: auto.

sidebar-with-ignore

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@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...

Extracted all shared card-building logic into src/card.ts in 87060fb. Both tooltip and panel now use:

  • buildCardContent() — shared DOM construction (message + action buttons + ignore button) with context-specific callbacks
  • setAccentColor() — shared --harper-kind-color CSS property setter
  • findDiagnostic() / ignoreDiagnostic() — shared diagnostic lookup and removal logic
  • cardContentCSS() — shared base CSS for .harper-msg, .harper-btn, .harper-ignore, .harper-actions (including dark mode)

Tooltip and panel only retain their own container-specific CSS with size overrides (e.g. .harper-card .harper-btn { font-size: 12px } vs .harper-pane-item .harper-btn { font-size: 11px }). Net ~195 lines removed.

@cyanzhong
Copy link
Contributor

@copilot It looks like we should hide "Ignore" in sidebar mode, because we have fixed width:

I am seeing strange things like this:

image

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.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot It looks like we should hide "Ignore" in sidebar mode, because we have fixed width:

I am seeing strange things like this:
...

Fixed in ddcf45e:

  1. Ignore button removed from sidebar cardsbuildCardContent() now accepts a showIgnore option (default: true). Panel passes showIgnore: false; tooltip uses the default.
  2. No more line wrapping — pane actions row uses flex-wrap: nowrap + overflow: hidden.
  3. Self-review complete — removed unused ignoreDiagnostic import and .harper-pane-item .harper-ignore CSS override from panel.ts, updated doc comments in card.ts.

All 35 tests pass, lint clean, no CodeQL alerts.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts and refactors tooltip.ts to use it.
  • Improves diagnostic robustness by mapping Diagnostic.from/to through 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.

@cyanzhong cyanzhong force-pushed the copilot/add-dedicated-proofreading-pane branch from af322e4 to 5904151 Compare March 15, 2026 07:34
@cyanzhong
Copy link
Contributor

@copilot Final self-review.

Copy link
Contributor Author

Copilot AI commented Mar 15, 2026

@copilot Final self-review.

Self-review complete in 362b3fa. Three issues fixed:

  1. Deduplicated fallback colorsfallback/fallbackDark now exported from styling.ts and imported in card.ts instead of being defined in both files.
  2. Pane auto-closes — when the last card is dismissed via suggestion acceptance, dismissCard() dispatches togglePanelEffect.of(false) to close the pane with its slide-out animation (previously showed an empty state instead).
  3. Reused findDiagnostic() — pane card click handler now uses the shared findDiagnostic() from card.ts instead of duplicating the lookup logic inline.

All 35 tests pass, lint clean, no CodeQL alerts.

@cyanzhong cyanzhong changed the title Add dedicated Review Problems pane Add "Review Problems" pane Mar 15, 2026
@cyanzhong cyanzhong merged commit c4e63d9 into main Mar 15, 2026
1 check passed
@cyanzhong cyanzhong deleted the copilot/add-dedicated-proofreading-pane branch March 15, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dedicated proofreading pane

3 participants