Skip to content

sessions: share status progress indicator between header and list#319995

Merged
sandy081 merged 4 commits into
mainfrom
sessions/shared-status-indicator
Jun 4, 2026
Merged

sessions: share status progress indicator between header and list#319995
sandy081 merged 4 commits into
mainfrom
sessions/shared-status-indicator

Conversation

@sandy081
Copy link
Copy Markdown
Member

@sandy081 sandy081 commented Jun 4, 2026

What

The session header (top of a session view) showed a static codicon for in-progress / needs-input sessions, while the sessions list showed the animated pixel spinner. This makes the header render the same animated progress indicator as the list, and extracts the shared logic so the two surfaces can't drift apart.

Changes

  • New src/vs/sessions/browser/sessionStatusIcon.ts (sessions core layer):
    • getSessionStatusIcon(...) — status → codicon mapping (previously duplicated verbatim).
    • getSessionStatusIndicator(...) — resolves a discriminated SessionStatusIndicator (spinner vs icon) carrying the spinner variant, a cacheKey, and a ready-to-apply CSS color. Centralizes the spinner-vs-codicon decision, variant selection (grid for in-progress, ring for needs-input), and colors.
  • sessionHeader.ts — now renders the animated pixel spinner for in-progress / needs-input (grid/ring variants), falling back to the static codicon when reduced motion is requested (IAccessibilityService). Re-renders react to reduced-motion changes via observableSignalFromEvent. Icon glyph/variant is cached so unrelated observable updates don't restart the animation.
  • sessionsList.ts — refactored its icon block to use the shared getSessionStatusIndicator; removed the duplicated getStatusIcon method and local spinner-key constants.

Why this layer

browser/parts/ (core) must not import from contrib/ per src/vs/sessions/LAYERS.md — which is exactly why the logic was originally copied. Placing the helper in sessions/browser/ lets both the core SessionHeader and the contrib SessionsList reuse it without violating layering.

Notes for reviewers

  • Each call site keeps its own DOM rendering (the list's crossfade swap vs. the header's simple reset) since those genuinely differ — only the shared decision logic is centralized.
  • Behavior parity: identical statuses, variants, and theme colors as before.

Validation

  • npm run compile-check-ts-native
  • npm run valid-layers-check
  • eslint on changed files ✅

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The session header showed a static codicon for in-progress / needs-input
sessions while the sessions list showed the animated pixel spinner. Make
the header render the same spinner, and extract the shared status-icon
logic into sessions/browser/sessionStatusIcon.ts so both surfaces stay in
sync without the core browser layer importing from contrib.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 4, 2026 19:53
@sandy081 sandy081 self-assigned this Jun 4, 2026
@sandy081 sandy081 added this to the 1.124.0 milestone Jun 4, 2026
@sandy081 sandy081 enabled auto-merge (squash) June 4, 2026 19:53
@vs-code-engineering
Copy link
Copy Markdown
Contributor

vs-code-engineering Bot commented Jun 4, 2026

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@benibenj

Matched files:

  • src/vs/sessions/browser/parts/sessionHeader.ts

@lszomoru

Matched files:

  • src/vs/sessions/services/sessions/browser/sessionsListModelService.ts

Copy link
Copy Markdown
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

Centralizes session status → visual indicator logic so the session header and sessions list render the same progress UI (animated pixel spinner when motion is allowed; codicon fallback when reduced motion is requested), preventing drift between the two surfaces.

Changes:

  • Introduces a shared getSessionStatusIndicator(...) helper (and underlying icon mapping) in src/vs/sessions/browser/ to unify spinner-vs-codicon decisions, variants, and colors.
  • Refactors the sessions list row renderer to consume the shared indicator instead of duplicating icon/spinner logic.
  • Updates the session header to render the same animated pixel spinner as the list, re-rendering on reduced-motion changes while caching the current glyph/variant to avoid restarting animations unnecessarily.
Show a summary per file
File Description
src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts Replaces inline status icon/spinner logic with shared getSessionStatusIndicator output while preserving existing crossfade behavior.
src/vs/sessions/browser/sessionStatusIcon.ts New shared status→indicator helper (codicon mapping + discriminated indicator result including spinner variant, cache key, and CSS color).
src/vs/sessions/browser/parts/sessionHeader.ts Uses shared indicator to render animated pixel spinner in header (with reduced-motion fallback), and avoids restarting animation on unrelated observable updates.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 4

Comment thread src/vs/sessions/browser/sessionStatusIcon.ts Outdated
Comment thread src/vs/sessions/browser/sessionStatusIcon.ts Outdated
Comment thread src/vs/sessions/browser/sessionStatusIcon.ts Outdated
Comment thread src/vs/sessions/browser/sessionStatusIcon.ts Outdated
sandy081 and others added 3 commits June 4, 2026 22:04
…s-indicator

# Conflicts:
#	src/vs/sessions/browser/parts/sessionHeader.ts
#	src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts
Encapsulate all status-icon rendering (spinner vs. codicon, cross-fade,
reduced-motion reactivity, pulse) in a reusable SessionStatusIcon widget
so each surface just hosts it and the model service only provides data.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address code review feedback: the status icon fallback serves the sessions
list, session header, and picker, not just the list.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sandy081 sandy081 merged commit d4f23d0 into main Jun 4, 2026
39 of 40 checks passed
@sandy081 sandy081 deleted the sessions/shared-status-indicator branch June 4, 2026 21:13
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.

3 participants