From 9c7044880b7ee297a12222f5b0e613b50bb278c0 Mon Sep 17 00:00:00 2001 From: Keith Daulton Date: Wed, 25 Jun 2025 15:00:45 -0400 Subject: [PATCH 1/3] Improves custom graph color creation - moves graph theme from GraphAppState to graph wrapper - reduces JS-based color computing - removes computing colors of CSS custom properties - renames components for clarity --- src/system/color.ts | 8 + ...h-wrapper.react.tsx => gl-graph.react.tsx} | 7 +- .../{graph-wrapper-element.ts => gl-graph.ts} | 6 +- .../plus/graph/graph-wrapper/graph-wrapper.ts | 178 +++++++++++++++++- src/webviews/apps/plus/graph/graph.ts | 124 +----------- src/webviews/apps/plus/graph/stateProvider.ts | 5 - src/webviews/plus/graph/protocol.ts | 8 +- 7 files changed, 191 insertions(+), 145 deletions(-) rename src/webviews/apps/plus/graph/graph-wrapper/{graph-wrapper.react.tsx => gl-graph.react.tsx} (98%) rename src/webviews/apps/plus/graph/graph-wrapper/{graph-wrapper-element.ts => gl-graph.ts} (98%) diff --git a/src/system/color.ts b/src/system/color.ts index b5daa52d18b49..d2b8f5c912844 100644 --- a/src/system/color.ts +++ b/src/system/color.ts @@ -805,3 +805,11 @@ function _parseHexDigit(charCode: CharCode): number { } return 0; } + +export function getCssMixedColorValue(color1: string, color2: string, percent: number): string { + return `color-mix(in srgb, ${color1} ${percent}%, ${color2})`; +} + +export function getCssOpacityColorValue(color: string, percent: number): string { + return getCssMixedColorValue(color, 'transparent', percent); +} diff --git a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.react.tsx b/src/webviews/apps/plus/graph/graph-wrapper/gl-graph.react.tsx similarity index 98% rename from src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.react.tsx rename to src/webviews/apps/plus/graph/graph-wrapper/gl-graph.react.tsx index 58c13153905d3..79d535970408f 100644 --- a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.react.tsx +++ b/src/webviews/apps/plus/graph/graph-wrapper/gl-graph.react.tsx @@ -1,5 +1,6 @@ import type { CommitType, + CssVariables, ExcludeRefsById, ExternalIconKeys, GetExternalIcon, @@ -59,7 +60,7 @@ export type GraphWrapperProps = Pick< | 'rowsStatsLoading' | 'workingTreeStats' > & - Pick; + Pick & { theming?: GraphWrapperTheming }; export interface GraphWrapperEvents { onChangeColumns?: (columns: GraphColumnsConfig) => void; @@ -197,6 +198,8 @@ interface GraphWrapperAPI { setRef: (refObject: GraphContainer) => void; } +export type GraphWrapperTheming = { cssVariables: CssVariables; themeOpacityFactor: number }; + export type GraphWrapperSubscriberProps = GraphWrapperProps & GraphWrapperAPI; export type GraphWrapperInitProps = GraphWrapperProps & GraphWrapperEvents & @@ -206,7 +209,7 @@ export type GraphWrapperInitProps = GraphWrapperProps & const emptyRows: GraphRow[] = []; -export const GraphWrapperReact = memo((initProps: GraphWrapperInitProps) => { +export const GlGraphReact = memo((initProps: GraphWrapperInitProps) => { const [graph, _graphRef] = useState(null); const [context, setContext] = useState(initProps.context); const [props, setProps] = useState(initProps); diff --git a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper-element.ts b/src/webviews/apps/plus/graph/graph-wrapper/gl-graph.ts similarity index 98% rename from src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper-element.ts rename to src/webviews/apps/plus/graph/graph-wrapper/gl-graph.ts index f4a1143ac73bd..365bfbf138524 100644 --- a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper-element.ts +++ b/src/webviews/apps/plus/graph/graph-wrapper/gl-graph.ts @@ -12,8 +12,8 @@ import type { GraphMissingRefsMetadata, GraphRefMetadataItem, } from '../../../../plus/graph/protocol'; -import type { GraphWrapperInitProps, GraphWrapperProps, GraphWrapperSubscriberProps } from './graph-wrapper.react'; -import { GraphWrapperReact } from './graph-wrapper.react'; +import type { GraphWrapperInitProps, GraphWrapperProps, GraphWrapperSubscriberProps } from './gl-graph.react'; +import { GlGraphReact } from './gl-graph.react'; /** * A LitElement web component that encapsulates the GraphWrapperReact component. @@ -146,7 +146,7 @@ export class GlGraph extends LitElement { // Mount the React component this.reactRoot.render( - createElement(GraphWrapperReact, { + createElement(GlGraphReact, { setRef: this.setRef, subscriber: this.setReactStateProvider, diff --git a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.ts b/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.ts index 5ab0a7dceb597..9738a41926104 100644 --- a/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.ts +++ b/src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.ts @@ -1,12 +1,15 @@ +/*global document window*/ import type GraphContainer from '@gitkraken/gitkraken-components'; -import type { GraphRow, GraphZoneType } from '@gitkraken/gitkraken-components'; +import type { CssVariables, GraphRow, GraphZoneType } from '@gitkraken/gitkraken-components'; import { consume } from '@lit/context'; import { SignalWatcher } from '@lit-labs/signals'; import { html, LitElement } from 'lit'; -import { customElement, query } from 'lit/decorators.js'; +import { customElement, query, state } from 'lit/decorators.js'; import { ifDefined } from 'lit/directives/if-defined.js'; import type { GitGraphRowType } from '../../../../../git/models/graph'; import { filterMap } from '../../../../../system/array'; +import { getCssMixedColorValue, getCssOpacityColorValue, getCssVariable } from '../../../../../system/color'; +import { debounce } from '../../../../../system/function/debounce'; import { DoubleClickedCommandType, GetMissingAvatarsCommand, @@ -20,10 +23,28 @@ import type { CustomEventType } from '../../../shared/components/element'; import { ipcContext } from '../../../shared/contexts/ipc'; import type { TelemetryContext } from '../../../shared/contexts/telemetry'; import { telemetryContext } from '../../../shared/contexts/telemetry'; +import type { Disposable } from '../../../shared/events'; +import type { ThemeChangeEvent } from '../../../shared/theme'; +import { onDidChangeTheme } from '../../../shared/theme'; import { stateContext } from '../context'; import { graphStateContext } from '../stateProvider'; -import type { GlGraph } from './graph-wrapper-element'; -import './graph-wrapper-element'; +import type { GlGraph } from './gl-graph'; +import type { GraphWrapperTheming } from './gl-graph.react'; +import './gl-graph'; + +// These properties in the DOM are auto-generated by VS Code from our `contributes.colors` in package.json +const graphLaneThemeColors = new Map([ + ['--vscode-gitlens-graphLane1Color', '#15a0bf'], + ['--vscode-gitlens-graphLane2Color', '#0669f7'], + ['--vscode-gitlens-graphLane3Color', '#8e00c2'], + ['--vscode-gitlens-graphLane4Color', '#c517b6'], + ['--vscode-gitlens-graphLane5Color', '#d90171'], + ['--vscode-gitlens-graphLane6Color', '#cd0101'], + ['--vscode-gitlens-graphLane7Color', '#f25d2e'], + ['--vscode-gitlens-graphLane8Color', '#f2ca33'], + ['--vscode-gitlens-graphLane9Color', '#7bd938'], + ['--vscode-gitlens-graphLane10Color', '#2ece9d'], +]); declare global { // interface HTMLElementTagNameMap { @@ -57,6 +78,8 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) { return this; } + private disposables: Disposable[] = []; + @consume({ context: graphStateContext }) private readonly graphAppState!: typeof graphStateContext.__context__; @@ -77,6 +100,29 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) { this.ref = ref; }; + @state() + private theming?: GraphWrapperTheming; + + override connectedCallback(): void { + super.connectedCallback?.(); + + this.theming = this.getGraphTheming(); + this.disposables.push( + onDidChangeTheme( + debounce((e: ThemeChangeEvent) => { + this.theming = this.getGraphTheming(e); + }, 100), + ), + ); + } + + override disconnectedCallback(): void { + super.disconnectedCallback?.(); + + this.disposables.forEach(d => d.dispose()); + this.disposables = []; + } + override render() { const { graphAppState, hostState } = this; @@ -100,7 +146,7 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) { .rowsStats=${hostState.rowsStats} .searchResults=${graphAppState.searchResults} .selectedRows=${graphAppState.selectedRows} - .theming=${graphAppState.theming} + .theming=${this.theming} ?windowFocused=${hostState.windowFocused} .workingTreeStats=${hostState.workingTreeStats} @changecolumns=${this.onColumnsChanged} @@ -193,4 +239,126 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) { private onVisibleDaysChanged({ detail }: CustomEventType<'graph-changevisibledays'>) { this.dispatchEvent(new CustomEvent('gl-graph-change-visible-days', { detail: detail })); } + + private readonly themingDefaults: { cssVariables: CssVariables; themeOpacityFactor: number } = { + cssVariables: (() => { + const bgColor = getCssVariableValue('--color-background'); + const mixedGraphColors: CssVariables = {}; + let i = 0; + let color; + for (const [colorVar, colorDefault] of graphLaneThemeColors) { + color = getCssVariableValue(colorVar, { fallbackValue: colorDefault }); + mixedGraphColors[`--graph-color-${i}`] = color; + for (const mixInt of [15, 25, 45, 50]) { + mixedGraphColors[`--graph-color-${i}-bg${mixInt}`] = getCssMixedColorValue(bgColor, color, mixInt); + } + for (const mixInt of [10, 50]) { + mixedGraphColors[`--graph-color-${i}-f${mixInt}`] = getCssOpacityColorValue(color, mixInt); + } + i++; + } + return { + '--app__bg0': getCssVariableValue('--color-background'), + '--panel__bg0': getCssVariableValue('--color-graph-background'), + '--panel__bg1': getCssVariableValue('--color-graph-background2'), + '--section-border': getCssVariableValue('--color-graph-background2'), + '--selected-row': getCssVariableValue('--color-graph-selected-row'), + '--selected-row-border': 'none', + '--hover-row': getCssVariableValue('--color-graph-hover-row'), + '--hover-row-border': 'none', + '--scrollable-scrollbar-thickness': getCssVariableValue('--graph-column-scrollbar-thickness'), + '--scroll-thumb-bg': getCssVariableValue('--vscode-scrollbarSlider-background'), + '--scroll-marker-head-color': getCssVariableValue('--color-graph-scroll-marker-head'), + '--scroll-marker-upstream-color': getCssVariableValue('--color-graph-scroll-marker-upstream'), + '--scroll-marker-highlights-color': getCssVariableValue('--color-graph-scroll-marker-highlights'), + '--scroll-marker-local-branches-color': getCssVariableValue( + '--color-graph-scroll-marker-local-branches', + ), + '--scroll-marker-remote-branches-color': getCssVariableValue( + '--color-graph-scroll-marker-remote-branches', + ), + '--scroll-marker-stashes-color': getCssVariableValue('--color-graph-scroll-marker-stashes'), + '--scroll-marker-tags-color': getCssVariableValue('--color-graph-scroll-marker-tags'), + '--scroll-marker-selection-color': getCssVariableValue('--color-graph-scroll-marker-selection'), + '--scroll-marker-pull-requests-color': getCssVariableValue('--color-graph-scroll-marker-pull-requests'), + '--stats-added-color': getCssVariableValue('--color-graph-stats-added'), + '--stats-deleted-color': getCssVariableValue('--color-graph-stats-deleted'), + '--stats-files-color': getCssVariableValue('--color-graph-stats-files'), + '--stats-bar-border-radius': getCssVariableValue('--graph-stats-bar-border-radius'), + '--stats-bar-height': getCssVariableValue('--graph-stats-bar-height'), + '--text-selected': getCssVariableValue('--color-graph-text-selected'), + '--text-selected-row': getCssVariableValue('--color-graph-text-selected-row'), + '--text-hovered': getCssVariableValue('--color-graph-text-hovered'), + '--text-dimmed-selected': getCssVariableValue('--color-graph-text-dimmed-selected'), + '--text-dimmed': getCssVariableValue('--color-graph-text-dimmed'), + '--text-normal': getCssVariableValue('--color-graph-text-normal'), + '--text-secondary': getCssVariableValue('--color-graph-text-secondary'), + '--text-disabled': getCssVariableValue('--color-graph-text-disabled'), + '--text-accent': getCssVariableValue('--color-link-foreground'), + '--text-inverse': getCssVariableValue('--vscode-input-background'), + '--text-bright': getCssVariableValue('--vscode-input-background'), + ...mixedGraphColors, + }; + })(), + themeOpacityFactor: 1, + }; + + private getGraphTheming(e?: ThemeChangeEvent): GraphWrapperTheming { + // this will be called on theme updated as well as on config updated since it is dependent on the column colors from config changes and the background color from the theme + const computedStyle = e?.computedStyle ?? window.getComputedStyle(document.documentElement); + const bgColor = getCssVariableValue('--color-background', { computedStyle: computedStyle }); + + const mixedGraphColors: CssVariables = {}; + + let i = 0; + let color; + for (const [colorVar, colorDefault] of graphLaneThemeColors) { + color = getCssVariableValue(colorVar, { computedStyle: computedStyle, fallbackValue: colorDefault }); + + mixedGraphColors[`--column-${i}-color`] = getCssVariable(colorVar, computedStyle) || colorDefault; + + for (const mixInt of [15, 25, 45, 50]) { + mixedGraphColors[`--graph-color-${i}-bg${mixInt}`] = getCssMixedColorValue(bgColor, color, mixInt); + } + + i++; + } + + const isHighContrastTheme = + e?.isHighContrastTheme ?? + (document.body.classList.contains('vscode-high-contrast') || + document.body.classList.contains('vscode-high-contrast-light')); + + return { + cssVariables: { + ...this.themingDefaults.cssVariables, + '--selected-row-border': isHighContrastTheme + ? `1px solid ${getCssVariableValue('--color-graph-contrast-border', { computedStyle: computedStyle })}` + : 'none', + '--hover-row-border': isHighContrastTheme + ? `1px dashed ${getCssVariableValue('--color-graph-contrast-border', { computedStyle: computedStyle })}` + : 'none', + ...mixedGraphColors, + }, + themeOpacityFactor: + parseInt(getCssVariable('--graph-theme-opacity-factor', computedStyle)) || + this.themingDefaults.themeOpacityFactor, + }; + } +} + +function getCssVariableValue( + variable: string, + options?: { computedStyle?: CSSStyleDeclaration; fallbackValue?: string }, +): string { + const fallbackValue = options?.computedStyle + ? getCssVariable(variable, options?.computedStyle) + : options?.fallbackValue + ? options.fallbackValue + : undefined; + + if (fallbackValue) { + return `var(${variable}, ${fallbackValue})`; + } + return `var(${variable})`; } diff --git a/src/webviews/apps/plus/graph/graph.ts b/src/webviews/apps/plus/graph/graph.ts index ff1dc07b2ba1b..888bd7b9b6bd0 100644 --- a/src/webviews/apps/plus/graph/graph.ts +++ b/src/webviews/apps/plus/graph/graph.ts @@ -1,9 +1,7 @@ -/*global document window*/ -import type { CssVariables } from '@gitkraken/gitkraken-components'; import { provide } from '@lit/context'; import { html } from 'lit'; import { customElement, query, state } from 'lit/decorators.js'; -import { Color, getCssVariable, mix, opacity } from '../../../../system/color'; +import { Color } from '../../../../system/color'; import type { State } from '../../../plus/graph/protocol'; import type { StateProvider } from '../../shared/appHost'; import { GlAppHost } from '../../shared/appHost'; @@ -14,19 +12,6 @@ import { GraphAppState, graphStateContext, GraphStateProvider } from './statePro import './graph-app'; import './graph.scss'; -const graphLaneThemeColors = new Map([ - ['--vscode-gitlens-graphLane1Color', '#15a0bf'], - ['--vscode-gitlens-graphLane2Color', '#0669f7'], - ['--vscode-gitlens-graphLane3Color', '#8e00c2'], - ['--vscode-gitlens-graphLane4Color', '#c517b6'], - ['--vscode-gitlens-graphLane5Color', '#d90171'], - ['--vscode-gitlens-graphLane6Color', '#cd0101'], - ['--vscode-gitlens-graphLane7Color', '#f25d2e'], - ['--vscode-gitlens-graphLane8Color', '#f2ca33'], - ['--vscode-gitlens-graphLane9Color', '#7bd938'], - ['--vscode-gitlens-graphLane10Color', '#2ece9d'], -]); - @customElement('gl-graph-apphost') export class GraphAppHost extends GlAppHost { protected override createRenderRoot(): HTMLElement | DocumentFragment { @@ -162,116 +147,9 @@ export class GraphAppHost extends GlAppHost { '--branch-status-both-pill-background', c.luminance(branchStatusPillLuminance).toString(), ); - - const th = this.getGraphTheming(); - Object.entries(th.cssVariables).forEach(([property, value]) => { - rootStyle.setProperty(property, value.toString()); - }); - this.applyTheme(th); } protected override onWebviewVisibilityChanged(visible: boolean): void { this.appElement?.onWebviewVisibilityChanged(visible); } - - private applyTheme(theme: { cssVariables: CssVariables; themeOpacityFactor: number }) { - this._graphState.theming = theme; - } - - private getGraphTheming(): { cssVariables: CssVariables; themeOpacityFactor: number } { - // this will be called on theme updated as well as on config updated since it is dependent on the column colors from config changes and the background color from the theme - const computedStyle = window.getComputedStyle(document.documentElement); - const bgColor = getCssVariable('--color-background', computedStyle); - - const mixedGraphColors: CssVariables = {}; - - let i = 0; - let color; - for (const [colorVar, colorDefault] of graphLaneThemeColors) { - color = getCssVariable(colorVar, computedStyle) || colorDefault; - - mixedGraphColors[`--column-${i}-color`] = color; - - mixedGraphColors[`--graph-color-${i}`] = color; - for (const mixInt of [15, 25, 45, 50]) { - mixedGraphColors[`--graph-color-${i}-bg${mixInt}`] = mix(bgColor, color, mixInt); - } - for (const mixInt of [10, 50]) { - mixedGraphColors[`--graph-color-${i}-f${mixInt}`] = opacity(color, mixInt); - } - - i++; - } - - const isHighContrastTheme = - document.body.classList.contains('vscode-high-contrast') || - document.body.classList.contains('vscode-high-contrast-light'); - - return { - cssVariables: { - '--app__bg0': bgColor, - '--panel__bg0': getCssVariable('--color-graph-background', computedStyle), - '--panel__bg1': getCssVariable('--color-graph-background2', computedStyle), - '--section-border': getCssVariable('--color-graph-background2', computedStyle), - - '--selected-row': getCssVariable('--color-graph-selected-row', computedStyle), - '--selected-row-border': isHighContrastTheme - ? `1px solid ${getCssVariable('--color-graph-contrast-border', computedStyle)}` - : 'none', - '--hover-row': getCssVariable('--color-graph-hover-row', computedStyle), - '--hover-row-border': isHighContrastTheme - ? `1px dashed ${getCssVariable('--color-graph-contrast-border', computedStyle)}` - : 'none', - - '--scrollable-scrollbar-thickness': getCssVariable('--graph-column-scrollbar-thickness', computedStyle), - '--scroll-thumb-bg': getCssVariable('--vscode-scrollbarSlider-background', computedStyle), - - '--scroll-marker-head-color': getCssVariable('--color-graph-scroll-marker-head', computedStyle), - '--scroll-marker-upstream-color': getCssVariable('--color-graph-scroll-marker-upstream', computedStyle), - '--scroll-marker-highlights-color': getCssVariable( - '--color-graph-scroll-marker-highlights', - computedStyle, - ), - '--scroll-marker-local-branches-color': getCssVariable( - '--color-graph-scroll-marker-local-branches', - computedStyle, - ), - '--scroll-marker-remote-branches-color': getCssVariable( - '--color-graph-scroll-marker-remote-branches', - computedStyle, - ), - '--scroll-marker-stashes-color': getCssVariable('--color-graph-scroll-marker-stashes', computedStyle), - '--scroll-marker-tags-color': getCssVariable('--color-graph-scroll-marker-tags', computedStyle), - '--scroll-marker-selection-color': getCssVariable( - '--color-graph-scroll-marker-selection', - computedStyle, - ), - '--scroll-marker-pull-requests-color': getCssVariable( - '--color-graph-scroll-marker-pull-requests', - computedStyle, - ), - - '--stats-added-color': getCssVariable('--color-graph-stats-added', computedStyle), - '--stats-deleted-color': getCssVariable('--color-graph-stats-deleted', computedStyle), - '--stats-files-color': getCssVariable('--color-graph-stats-files', computedStyle), - '--stats-bar-border-radius': getCssVariable('--graph-stats-bar-border-radius', computedStyle), - '--stats-bar-height': getCssVariable('--graph-stats-bar-height', computedStyle), - - '--text-selected': getCssVariable('--color-graph-text-selected', computedStyle), - '--text-selected-row': getCssVariable('--color-graph-text-selected-row', computedStyle), - '--text-hovered': getCssVariable('--color-graph-text-hovered', computedStyle), - '--text-dimmed-selected': getCssVariable('--color-graph-text-dimmed-selected', computedStyle), - '--text-dimmed': getCssVariable('--color-graph-text-dimmed', computedStyle), - '--text-normal': getCssVariable('--color-graph-text-normal', computedStyle), - '--text-secondary': getCssVariable('--color-graph-text-secondary', computedStyle), - '--text-disabled': getCssVariable('--color-graph-text-disabled', computedStyle), - - '--text-accent': getCssVariable('--color-link-foreground', computedStyle), - '--text-inverse': getCssVariable('--vscode-input-background', computedStyle), - '--text-bright': getCssVariable('--vscode-input-background', computedStyle), - ...mixedGraphColors, - }, - themeOpacityFactor: parseInt(getCssVariable('--graph-theme-opacity-factor', computedStyle)) || 1, - }; - } } diff --git a/src/webviews/apps/plus/graph/stateProvider.ts b/src/webviews/apps/plus/graph/stateProvider.ts index 86ad982becce2..91780c3165eda 100644 --- a/src/webviews/apps/plus/graph/stateProvider.ts +++ b/src/webviews/apps/plus/graph/stateProvider.ts @@ -1,4 +1,3 @@ -import type { CssVariables } from '@gitkraken/gitkraken-components'; import { ContextProvider, createContext } from '@lit/context'; import type { ReactiveControllerHost } from 'lit'; import type { SearchQuery } from '../../../../constants.search'; @@ -46,7 +45,6 @@ interface AppState { top: number; bottom: number; }; - theming?: { cssVariables: CssVariables; themeOpacityFactor: number }; } function getSearchResultModel(searchResults: State['searchResults']): { @@ -81,9 +79,6 @@ export class GraphAppState implements AppState { @signalObjectState() accessor visibleDays: AppState['visibleDays']; - @signalObjectState() - accessor theming: AppState['theming']; - @signalObjectState( { query: '' }, { diff --git a/src/webviews/plus/graph/protocol.ts b/src/webviews/plus/graph/protocol.ts index c715c3aca1096..eb00f463d1dbb 100644 --- a/src/webviews/plus/graph/protocol.ts +++ b/src/webviews/plus/graph/protocol.ts @@ -1,5 +1,4 @@ import type { - CssVariables, ExcludeByType, ExcludeRefsById, GraphColumnSetting, @@ -134,7 +133,6 @@ export interface State extends WebviewState { top: number; bottom: number; }; - theming?: { cssVariables: CssVariables; themeOpacityFactor: number }; } export interface BranchState extends GitTrackingState { @@ -220,11 +218,7 @@ export type GraphRowStats = RowStats; export type InternalNotificationType = 'didChangeTheme'; -export type UpdateStateCallback = ( - state: State, - type?: IpcNotification | InternalNotificationType, - themingChanged?: boolean, -) => void; +export type UpdateStateCallback = (state: State, type?: IpcNotification | InternalNotificationType) => void; // COMMANDS From 9574c4441aaa945d48a1140d7b50bc648c19afa4 Mon Sep 17 00:00:00 2001 From: Keith Daulton Date: Wed, 9 Jul 2025 13:50:38 -0400 Subject: [PATCH 2/3] Consolidates GraphAppState and GraphStateProvider --- src/webviews/apps/plus/graph/graph-app.ts | 42 ++-- src/webviews/apps/plus/graph/graph-header.ts | 219 +++++++++--------- .../graph/graph-wrapper/gl-graph.react.tsx | 4 +- .../plus/graph/graph-wrapper/graph-wrapper.ts | 56 +++-- src/webviews/apps/plus/graph/graph.ts | 20 +- src/webviews/apps/plus/graph/stateProvider.ts | 70 ++++-- src/webviews/apps/shared/appHost.ts | 5 +- .../apps/shared/components/signal-utils.ts | 11 +- 8 files changed, 220 insertions(+), 207 deletions(-) diff --git a/src/webviews/apps/plus/graph/graph-app.ts b/src/webviews/apps/plus/graph/graph-app.ts index 11af6583b8ceb..df3e6671979a6 100644 --- a/src/webviews/apps/plus/graph/graph-app.ts +++ b/src/webviews/apps/plus/graph/graph-app.ts @@ -13,7 +13,6 @@ import { ipcContext } from '../../shared/contexts/ipc'; import type { TelemetryContext } from '../../shared/contexts/telemetry'; import { telemetryContext } from '../../shared/contexts/telemetry'; import { emitTelemetrySentEvent } from '../../shared/telemetry'; -import { stateContext } from './context'; import type { GlGraphWrapper } from './graph-wrapper/graph-wrapper'; import type { GlGraphHover } from './hover/graphHover'; import type { GraphMinimapDaySelectedEventDetail } from './minimap/minimap'; @@ -36,11 +35,8 @@ export class GraphApp extends SignalWatcher(LitElement) { return this; } - @consume({ context: stateContext, subscribe: true }) - state!: typeof stateContext.__context__; - @consume({ context: graphStateContext, subscribe: true }) - graphApp!: typeof graphStateContext.__context__; + graphState!: typeof graphStateContext.__context__; @consume({ context: ipcContext }) private readonly _ipc!: typeof ipcContext.__context__; @@ -76,27 +72,33 @@ export class GraphApp extends SignalWatcher(LitElement) { @gl-select-commits=${this.handleHeaderSearchNavigation} >
- ${when(!this.state.allowed, () => html``)} + ${when( + !this.graphState.state.allowed, + () => html``, + )}
${when( - this.state.config?.minimap !== false, + this.graphState.state.config?.minimap !== false, () => html` `, )} - ${when(this.state.config?.sidebar, () => html``)} + ${when( + this.graphState.state.config?.sidebar, + () => html``, + )} ) { - if (!this.state.rows) return; + if (!this.graphState.state.rows) return; let { sha } = e.detail; if (sha == null) { @@ -127,7 +129,7 @@ export class GraphApp extends SignalWatcher(LitElement) { if (date == null) return; // Find closest row to the date - const closest = this.state.rows.reduce((prev, curr) => { + const closest = this.graphState.state.rows.reduce((prev, curr) => { return Math.abs(curr.date - date) < Math.abs(prev.date - date) ? curr : prev; }); sha = closest.sha; @@ -161,7 +163,7 @@ export class GraphApp extends SignalWatcher(LitElement) { } private handleGraphVisibleDaysChanged({ detail }: CustomEventType<'gl-graph-change-visible-days'>) { - this.graphApp.visibleDays = detail; + this.graphState.visibleDays = detail; } private handleGraphRowContextMenu(_e: CustomEventType<'gl-graph-row-context-menu'>) { diff --git a/src/webviews/apps/plus/graph/graph-header.ts b/src/webviews/apps/plus/graph/graph-header.ts index ef0fe24ef7210..036990335958b 100644 --- a/src/webviews/apps/plus/graph/graph-header.ts +++ b/src/webviews/apps/plus/graph/graph-header.ts @@ -48,7 +48,6 @@ import type { TelemetryContext } from '../../shared/contexts/telemetry'; import { telemetryContext } from '../../shared/contexts/telemetry'; import { emitTelemetrySentEvent } from '../../shared/telemetry'; import { ruleStyles } from '../shared/components/vscode.css'; -import { stateContext } from './context'; import { graphStateContext } from './stateProvider'; import { actionButton, linkBase } from './styles/graph.css'; import { graphHeaderControlStyles, progressStyles, repoHeaderStyles, titlebarStyles } from './styles/header.css'; @@ -129,27 +128,25 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { @consume({ context: telemetryContext as { __context__: TelemetryContext } }) _telemetry!: TelemetryContext; - @consume({ context: stateContext, subscribe: true }) - hostState!: typeof stateContext.__context__; - - @consume({ context: graphStateContext }) - appState!: typeof graphStateContext.__context__; + @consume({ context: graphStateContext, subscribe: true }) + graphState!: typeof graphStateContext.__context__; @state() private aiAllowed = true; get hasFilters() { - if (this.hostState.config?.onlyFollowFirstParent) return true; - if (this.hostState.excludeTypes == null) return false; + if (this.graphState.state.config?.onlyFollowFirstParent) return true; + if (this.graphState.state.excludeTypes == null) return false; - return Object.values(this.hostState.excludeTypes).includes(true); + return Object.values(this.graphState.state.excludeTypes).includes(true); } get excludeRefs() { - return Object.values(this.hostState.excludeRefs ?? {}).sort(compareGraphRefOpts); + return Object.values(this.graphState.state.excludeRefs ?? {}).sort(compareGraphRefOpts); } override updated(changedProperties: PropertyValues): void { - this.aiAllowed = (this.hostState.config?.aiEnabled ?? true) && (this.hostState.orgSettings?.ai ?? true); + this.aiAllowed = + (this.graphState.state.config?.aiEnabled ?? true) && (this.graphState.state.orgSettings?.ai ?? true); super.updated(changedProperties); } @@ -179,7 +176,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } private onSearchOpenInView() { - this._ipc.sendCommand(SearchOpenInViewCommand, { search: { ...this.appState.filter } }); + this._ipc.sendCommand(SearchOpenInViewCommand, { search: { ...this.graphState.filter } }); } private onExcludeTypesChanged(key: keyof GraphExcludeTypes, value: boolean) { @@ -193,7 +190,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { private _activeRowInfoCache: { row: string; info: { date: number; id: string } } | undefined; private getActiveRowInfo(): { date: number; id: string } | undefined { - const { activeRow } = this.appState; + const { activeRow } = this.graphState; if (activeRow == null) return undefined; if (this._activeRowInfoCache?.row === activeRow) return this._activeRowInfoCache?.info; @@ -293,13 +290,13 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } private _searchPositionSignal = computed(() => { - const { searchResults } = this.appState; - if (searchResults?.ids == null || !this.appState.filter.query) return 0; + const { searchResults } = this.graphState; + if (searchResults?.ids == null || !this.graphState.filter.query) return 0; const id = this.getActiveRowInfo()?.id; let searchIndex = id ? searchResults.ids[id]?.i : undefined; if (searchIndex == null) { - ({ index: searchIndex } = this.getClosestSearchResultIndex(searchResults, { ...this.appState.filter })); + ({ index: searchIndex } = this.getClosestSearchResultIndex(searchResults, { ...this.graphState.filter })); } return searchIndex < 1 ? 1 : searchIndex + 1; }); @@ -309,7 +306,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } get searchValid() { - return this.appState.filter.query.length > 2; + return this.graphState.filter.query.length > 2; } handleFilterChange(e: CustomEvent) { @@ -331,7 +328,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { case 'stashes': case 'tags': { const key = $el.value satisfies keyof GraphExcludeTypes; - const currentFilter = this.hostState.excludeTypes?.[key]; + const currentFilter = this.graphState.state.excludeTypes?.[key]; if ((currentFilter == null && checked) || (currentFilter != null && currentFilter !== checked)) { this.onExcludeTypesChanged(key, checked); } @@ -351,33 +348,33 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } async handleSearch() { - this.appState.searching = this.searchValid; + this.graphState.searching = this.searchValid; if (!this.searchValid) { - this.appState.searchResultsResponse = undefined; + this.graphState.searchResultsResponse = undefined; } try { const rsp = await this._ipc.sendRequest(SearchRequest, { - search: this.searchValid ? { ...this.appState.filter } : undefined /*limit: options?.limit*/, + search: this.searchValid ? { ...this.graphState.filter } : undefined /*limit: options?.limit*/, }); if (rsp.search && rsp.results) { this.searchEl.logSearch(rsp.search); } - this.appState.searchResultsResponse = rsp.results; + this.graphState.searchResultsResponse = rsp.results; if (rsp.selectedRows != null) { - this.appState.selectedRows = rsp.selectedRows; + this.graphState.selectedRows = rsp.selectedRows; } } catch { - this.appState.searchResultsResponse = undefined; + this.graphState.searchResultsResponse = undefined; } - this.appState.searching = false; + this.graphState.searching = false; } @debounce(250) private handleSearchInput(e: CustomEvent) { - this.appState.filter = e.detail; + this.graphState.filter = e.detail; void this.handleSearch(); } @@ -389,9 +386,9 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { more: options?.more, }); - this.appState.searchResultsResponse = rsp.results; + this.graphState.searchResultsResponse = rsp.results; if (rsp.selectedRows != null) { - this.appState.selectedRows = rsp.selectedRows; + this.graphState.selectedRows = rsp.selectedRows; } return rsp; @@ -401,7 +398,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } private async handleSearchNavigation(e: CustomEvent) { - let { searchResults } = this.appState; + let { searchResults } = this.graphState; if (searchResults == null) return; const direction = e.detail?.direction ?? 'next'; @@ -422,7 +419,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { next = direction === 'next'; ({ index: searchIndex, id } = this.getClosestSearchResultIndex( searchResults, - { ...this.appState.filter }, + { ...this.graphState.filter }, next, )); } @@ -435,13 +432,13 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { // Indicates a boundary and we need to load more results if (searchIndex === -1) { if (next) { - if (this.appState.filter.query && searchResults?.paging?.hasMore) { - this.appState.searching = true; + if (this.graphState.filter.query && searchResults?.paging?.hasMore) { + this.graphState.searching = true; let moreResults; try { - moreResults = await this.onSearchPromise?.({ ...this.appState.filter }, { more: true }); + moreResults = await this.onSearchPromise?.({ ...this.graphState.filter }, { more: true }); } finally { - this.appState.searching = false; + this.graphState.searching = false; } if (moreResults?.results != null && !('error' in moreResults.results)) { if (count < moreResults.results.count) { @@ -457,14 +454,17 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } else { searchIndex = 0; } - // this.appState.filter != null seems noop - } else if (direction === 'last' && this.appState.filter != null && searchResults?.paging?.hasMore) { - this.appState.searching = true; + // this.graphState.filter != null seems noop + } else if (direction === 'last' && this.graphState.filter != null && searchResults?.paging?.hasMore) { + this.graphState.searching = true; let moreResults; try { - moreResults = await this.onSearchPromise({ ...this.appState.filter }, { limit: 0, more: true }); + moreResults = await this.onSearchPromise( + { ...this.graphState.filter }, + { limit: 0, more: true }, + ); } finally { - this.appState.searching = false; + this.graphState.searching = false; } if (moreResults?.results != null && !('error' in moreResults.results)) { if (count < moreResults.results.count) { @@ -484,10 +484,10 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { if (id != null) break; } - this.appState.searchResultsHidden = true; + this.graphState.searchResultsHidden = true; searchIndex = this.getNextOrPreviousSearchResultIndex(searchIndex, next, searchResults, { - ...this.appState.filter, + ...this.graphState.filter, }); } @@ -516,13 +516,13 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { if (promise == null) { let timeout: ReturnType | undefined = setTimeout(() => { timeout = undefined; - this.appState.loading = true; + this.graphState.loading = true; }, 500); const ensureCore = async () => { const e = await this.onEnsureRowPromise(id, false); if (timeout == null) { - this.appState.loading = false; + this.graphState.loading = false; } else { clearTimeout(timeout); } @@ -555,7 +555,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } handleMinimapToggled() { - this.changeGraphConfiguration({ minimap: !this.hostState.config?.minimap }); + this.changeGraphConfiguration({ minimap: !this.graphState.state.config?.minimap }); } private changeGraphConfiguration(changes: UpdateGraphConfigurationParams['changes']) { @@ -563,30 +563,30 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { } private handleMinimapDataTypeChanged(e: Event) { - if (this.hostState.config == null) return; + if (this.graphState.state.config == null) return; const $el = e.target as RadioGroup; const minimapDataType = $el.value === 'lines' ? 'lines' : 'commits'; - if (this.hostState.config.minimapDataType === minimapDataType) return; + if (this.graphState.state.config.minimapDataType === minimapDataType) return; this.changeGraphConfiguration({ minimapDataType: minimapDataType }); } private handleMinimapAdditionalTypesChanged(e: Event) { - if (this.hostState.config?.minimapMarkerTypes == null) return; + if (this.graphState.state.config?.minimapMarkerTypes == null) return; const $el = e.target as HTMLInputElement; const value = $el.value as GraphMinimapMarkerTypes; if ($el.checked) { - if (!this.hostState.config.minimapMarkerTypes.includes(value)) { - const minimapMarkerTypes = [...this.hostState.config.minimapMarkerTypes, value]; + if (!this.graphState.state.config.minimapMarkerTypes.includes(value)) { + const minimapMarkerTypes = [...this.graphState.state.config.minimapMarkerTypes, value]; this.changeGraphConfiguration({ minimapMarkerTypes: minimapMarkerTypes }); } } else { - const index = this.hostState.config.minimapMarkerTypes.indexOf(value); + const index = this.graphState.state.config.minimapMarkerTypes.indexOf(value); if (index !== -1) { - const minimapMarkerTypes = [...this.hostState.config.minimapMarkerTypes]; + const minimapMarkerTypes = [...this.graphState.state.config.minimapMarkerTypes]; minimapMarkerTypes.splice(index, 1); this.changeGraphConfiguration({ minimapMarkerTypes: minimapMarkerTypes }); } @@ -613,17 +613,19 @@ export class GlGraphHeader extends SignalWatcher(LitElement) { private readonly searchEl!: GlSearchBox; override render() { - const repo = this.hostState.repositories?.find(repo => repo.id === this.hostState.selectedRepository); - const { searchResults } = this.appState; + const repo = this.graphState.state.repositories?.find( + repo => repo.id === this.graphState.state.selectedRepository, + ); + const { searchResults } = this.graphState; - const hasMultipleRepositories = (this.hostState.repositories?.length ?? 0) > 1; + const hasMultipleRepositories = (this.graphState.state.repositories?.length ?? 0) > 1; return cache( html`
${when( - this.hostState.allowed && repo, + this.graphState.state.allowed && repo, () => html` ${when( - this.hostState.branchState?.pr, + this.graphState.state.branchState?.pr, pr => html`
${when( - this.hostState.allowed && - this.hostState.workingTreeStats != null && - (this.hostState.workingTreeStats.hasConflicts || - this.hostState.workingTreeStats.pausedOpStatus), + this.graphState.state.allowed && + this.graphState.state.workingTreeStats != null && + (this.graphState.state.workingTreeStats.hasConflicts || + this.graphState.state.workingTreeStats.pausedOpStatus), () => html`
`, )} ${when( - this.hostState.allowed, + this.graphState.state.allowed, () => html`
@@ -861,7 +864,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) {
${when( - this.graphState.state.allowed && - this.graphState.state.workingTreeStats != null && - (this.graphState.state.workingTreeStats.hasConflicts || - this.graphState.state.workingTreeStats.pausedOpStatus), + this.graphState.allowed && + this.graphState.workingTreeStats != null && + (this.graphState.workingTreeStats.hasConflicts || + this.graphState.workingTreeStats.pausedOpStatus), () => html`
`, )} ${when( - this.graphState.state.allowed, + this.graphState.allowed, () => html`
@@ -864,7 +860,7 @@ export class GlGraphHeader extends SignalWatcher(LitElement) {