From 7bd2924c0ff75bafb52335e20d1e54ae7c607970 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Fri, 21 Oct 2022 10:35:00 -0400 Subject: [PATCH] Centralize popup manager to be accessed through the app-store --- app/src/lib/app-state.ts | 2 ++ app/src/lib/menu-update.ts | 3 +- app/src/lib/multi-commit-operation.ts | 5 ++- app/src/lib/popup-manager.ts | 8 ++--- app/src/lib/stores/app-store.ts | 47 ++++++++++++++++++--------- app/src/ui/app.tsx | 5 ++- app/src/ui/index.tsx | 5 ++- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/app/src/lib/app-state.ts b/app/src/lib/app-state.ts index 952da275d1..bd996cb09f 100644 --- a/app/src/lib/app-state.ts +++ b/app/src/lib/app-state.ts @@ -46,6 +46,7 @@ import { MultiCommitOperationStep, } from '../models/multi-commit-operation' import { IChangesetData } from './git' +import { Popup } from '../models/popup' export enum SelectionType { Repository, @@ -114,6 +115,7 @@ export interface IAppState { readonly showWelcomeFlow: boolean readonly focusCommitMessage: boolean + readonly currentPopup: Popup | null readonly currentFoldout: Foldout | null readonly currentBanner: Banner | null diff --git a/app/src/lib/menu-update.ts b/app/src/lib/menu-update.ts index c4825b0de1..9f0c644679 100644 --- a/app/src/lib/menu-update.ts +++ b/app/src/lib/menu-update.ts @@ -11,7 +11,6 @@ import { updateMenuState as ipcUpdateMenuState } from '../ui/main-process-proxy' import { AppMenu, MenuItem } from '../models/app-menu' import { hasConflictedFiles } from './status' import { findContributionTargetDefaultBranch } from './branch' -import { popupManager } from './popup-manager' export interface IMenuItemState { readonly enabled?: boolean @@ -364,7 +363,7 @@ function getRepositoryMenuBuilder(state: IAppState): MenuStateBuilder { } function getMenuState(state: IAppState): Map { - if (popupManager.isAPopupOpen) { + if (state.currentPopup) { return getAllMenusDisabledBuilder().state } diff --git a/app/src/lib/multi-commit-operation.ts b/app/src/lib/multi-commit-operation.ts index 8694cdf0ae..c1799a1dd1 100644 --- a/app/src/lib/multi-commit-operation.ts +++ b/app/src/lib/multi-commit-operation.ts @@ -4,10 +4,8 @@ import { conflictSteps, MultiCommitOperationStepKind, } from '../models/multi-commit-operation' -import { PopupType } from '../models/popup' import { TipState } from '../models/tip' import { IMultiCommitOperationState, IRepositoryState } from './app-state' -import { popupManager } from './popup-manager' /** * Setup the multi commit operation state when the user needs to select a branch as the @@ -40,10 +38,11 @@ export function getMultiCommitOperationChooseBranchStep( } export function isConflictsFlow( + isMultiCommitOperationPopupOpen: boolean, multiCommitOperationState: IMultiCommitOperationState | null ): boolean { return ( - popupManager.arePopupsOfType(PopupType.MultiCommitOperation) && + isMultiCommitOperationPopupOpen && multiCommitOperationState !== null && conflictSteps.includes(multiCommitOperationState.step.kind) ) diff --git a/app/src/lib/popup-manager.ts b/app/src/lib/popup-manager.ts index f2f7ce3047..97e611253b 100644 --- a/app/src/lib/popup-manager.ts +++ b/app/src/lib/popup-manager.ts @@ -18,15 +18,15 @@ export class PopupManager { /** * Returns the last popup added to the stack. */ - public get currentPopup(): Popup | undefined { - return this.popupStack.at(-1) + public get currentPopup(): Popup | null { + return this.popupStack.at(-1) ?? null } /** * Returns whether there are any popups in the stack. */ public get isAPopupOpen(): boolean { - return this.currentPopup !== undefined + return this.currentPopup !== null } /** @@ -116,5 +116,3 @@ export class PopupManager { this.popupStack = this.popupStack.filter(p => p.type !== popupType) } } - -export const popupManager = new PopupManager() diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 4aae35489d..5353a20a0a 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -304,7 +304,7 @@ import { offsetFromNow } from '../offset-from' import { findContributionTargetDefaultBranch } from '../branch' import { ValidNotificationPullRequestReview } from '../valid-notification-pull-request-review' import { determineMergeability } from '../git/merge-tree' -import { popupManager } from '../popup-manager' +import { PopupManager } from '../popup-manager' const LastSelectedRepositoryIDKey = 'last-selected-repository-id' @@ -500,6 +500,9 @@ export class AppStore extends TypedBaseStore { private lastThankYou: ILastThankYou | undefined private showCIStatusPopover: boolean = false + /** A service for managing the stack of open popups */ + private popupManager = new PopupManager() + public constructor( private readonly gitHubUserStore: GitHubUserStore, private readonly cloningRepositoriesStore: CloningRepositoriesStore, @@ -639,7 +642,7 @@ export class AppStore extends TypedBaseStore { // If there is a currently open popup, don't do anything here. Since the // app can only show one popup at a time, we don't want to close the current // one in favor of the error we're about to show. - if (popupManager.isAPopupOpen) { + if (this.popupManager.isAPopupOpen) { return } @@ -904,6 +907,7 @@ export class AppStore extends TypedBaseStore { appIsFocused: this.appIsFocused, selectedState: this.getSelectedState(), signInState: this.signInStore.getState(), + currentPopup: this.popupManager.currentPopup, currentFoldout: this.currentFoldout, errors: this.errors, showWelcomeFlow: this.showWelcomeFlow, @@ -2505,7 +2509,13 @@ export class AppStore extends TypedBaseStore { this.currentBanner !== null && this.currentBanner.type === BannerType.ConflictsFound - if (displayingBanner || isConflictsFlow(multiCommitOperationState)) { + if ( + displayingBanner || + isConflictsFlow( + this.popupManager.arePopupsOfType(PopupType.MultiCommitOperation), + multiCommitOperationState + ) + ) { return } @@ -2592,7 +2602,12 @@ export class AppStore extends TypedBaseStore { */ private clearConflictsFlowVisuals(state: IRepositoryState) { const { multiCommitOperationState } = state - if (userIsStartingMultiCommitOperation(multiCommitOperationState)) { + if ( + userIsStartingMultiCommitOperation( + this.popupManager.currentPopup, + multiCommitOperationState + ) + ) { return } @@ -3475,19 +3490,19 @@ export class AppStore extends TypedBaseStore { // applicable on Windows where we draw a custom app menu. this._closeFoldout(FoldoutType.AppMenu) - popupManager.addPopup(popup) + this.popupManager.addPopup(popup) this.emitUpdate() } /** This shouldn't be called directly. See `Dispatcher`. */ public _closePopup(popupType?: PopupType) { - const currentPopup = popupManager.currentPopup - if (currentPopup === undefined) { + const currentPopup = this.popupManager.currentPopup + if (currentPopup === null) { return } if (popupType === undefined) { - popupManager.removePopup(currentPopup) + this.popupManager.removePopup(currentPopup) } else { if (currentPopup.type !== popupType) { return @@ -3497,7 +3512,7 @@ export class AppStore extends TypedBaseStore { this._completeOpenInDesktop(() => Promise.resolve(null)) } - popupManager.removePopupByType(popupType) + this.popupManager.removePopupByType(popupType) } this.emitUpdate() @@ -6491,12 +6506,11 @@ export class AppStore extends TypedBaseStore { path, (title, value, description) => { if ( - popupManager.currentPopup !== undefined && - popupManager.currentPopup.type === - PopupType.CreateTutorialRepository + this.popupManager.currentPopup?.type === + PopupType.CreateTutorialRepository ) { - popupManager.updatePopup({ - ...popupManager.currentPopup, + this.popupManager.updatePopup({ + ...this.popupManager.currentPopup, progress: { kind: 'generic', title, value, description }, }) this.emitUpdate() @@ -7483,13 +7497,14 @@ function getInitialAction( } function userIsStartingMultiCommitOperation( + currentPopup: Popup | null, state: IMultiCommitOperationState | null ) { - if (!popupManager.isAPopupOpen || state === null) { + if (currentPopup === null || state === null) { return false } - if (popupManager.currentPopup?.type !== PopupType.MultiCommitOperation) { + if (currentPopup.type !== PopupType.MultiCommitOperation) { return false } diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index e8fe72a6e2..8a97914cbf 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -159,7 +159,6 @@ import { UnreachableCommitsDialog } from './history/unreachable-commits-dialog' import { OpenPullRequestDialog } from './open-pull-request/open-pull-request-dialog' import { sendNonFatalException } from '../lib/helpers/non-fatal-exception' import { createCommitURL } from '../lib/commit-url' -import { popupManager } from '../lib/popup-manager' const MinuteInMilliseconds = 1000 * 60 const HourInMilliseconds = MinuteInMilliseconds * 60 @@ -216,7 +215,7 @@ export class App extends React.Component { * modal dialog such as the preferences, or an error dialog. */ private get isShowingModal() { - return popupManager.isAPopupOpen || this.state.errors.length > 0 + return this.state.currentPopup !== null || this.state.errors.length > 0 } /** @@ -1376,7 +1375,7 @@ export class App extends React.Component { return null } - const popup = popupManager.currentPopup + const popup = this.state.currentPopup if (!popup) { return null diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index 8f037dcfc9..2d883d5606 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -79,7 +79,6 @@ import { NotificationsStore } from '../lib/stores/notifications-store' import * as ipcRenderer from '../lib/ipc-renderer' import { migrateRendererGUID } from '../lib/get-renderer-guid' import { initializeRendererNotificationHandler } from '../lib/notifications/notification-handler' -import { popupManager } from '../lib/popup-manager' if (__DEV__) { installDevGlobals() @@ -145,8 +144,8 @@ const sendErrorWithContext = ( extra.currentBanner = currentState.currentBanner.type } - if (popupManager.currentPopup !== undefined) { - extra.currentPopup = `${popupManager.currentPopup.type}` + if (currentState.currentPopup !== null) { + extra.currentPopup = `${currentState.currentPopup.type}` } if (currentState.selectedState !== null) {