Skip to content

Add confirmation before erasing game history on import or position edit#621

Draft
benckx with Copilot wants to merge 3 commits into
masterfrom
copilot/add-confirmation-importing-moves
Draft

Add confirmation before erasing game history on import or position edit#621
benckx with Copilot wants to merge 3 commits into
masterfrom
copilot/add-confirmation-importing-moves

Conversation

Copilot AI commented May 13, 2026

Copy link
Copy Markdown
Contributor
  • Add isEmpty() method to MoveTreeWidget to expose whether the move tree has moves
  • Show confirmation dialog before "Import moves" when the move tree is not empty (contextual menu via importMovesCallback in analysis-board.js)
  • Show confirmation dialog before "Import moves" in the dropdown menu (move-history-drop-down-menu.js)
  • Show confirmation dialog before "Edit start position" in the dropdown menu (move-history-drop-down-menu.js)
  • Show confirmation dialog before "Edit start position" button in settings panel (settings-manager.js)
  • Extract UI.openWithConfirmation(shouldConfirm, confirmText, yesButtonText, openFn) helper to eliminate duplicated confirmation pattern across all call sites

…n if game history exists

Agent-Logs-Url: https://github.com/benckx/elephantchess/sessions/f9aad699-f886-4ce7-bed7-3e0b7ac83c5b

Co-authored-by: benckx <8626080+benckx@users.noreply.github.com>
Copilot AI changed the title [WIP] Add confirmation when importing moves with existing history Add confirmation before erasing game history on import or position edit May 13, 2026
Copilot AI requested a review from benckx May 13, 2026 12:35
Copilot finished work on behalf of benckx May 13, 2026 12:35
@benckx

benckx commented May 16, 2026

Copy link
Copy Markdown
Owner

@copilot how could we avoid this copy pasta?

    const openImportMovesModal = () => {
                UI.showModalByName(Modals.IMPORT_MOVES, () => {
                    new ImportMovesHandler(this.#boardGui, this.#moveTreeWidget);
                });
            };
            if (!this.#moveTreeWidget.isEmpty()) {
                const text = buildSimpleSpan('Importing moves will erase the current game history. Continue?');
                UI.showConfirmationModal(text, openImportMovesModal, 'import', () => UI.hideModal(null), 'cancel');
            } else {
                openImportMovesModal();
            }

… confirmation pattern

Agent-Logs-Url: https://github.com/benckx/elephantchess/sessions/d9103861-dd07-4c61-a975-f9615cbe5844

Co-authored-by: benckx <8626080+benckx@users.noreply.github.com>

Copilot AI commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

@copilot how could we avoid this copy pasta?

...

Extracted the pattern into a new UI.openWithConfirmation(shouldConfirm, confirmText, yesButtonText, openFn) helper (commit b14c63e). It handles the buildSimpleSpan + showConfirmationModal vs direct call logic internally, so each call site now reads as a single expression:

UI.openWithConfirmation(
    !moveTreeWidget.isEmpty(),
    'Importing moves will erase the current game history. Continue?',
    'import',
    () => UI.showModalByName(Modals.IMPORT_MOVES, () => new ImportMovesHandler(boardGui, moveTreeWidget))
);

Copilot finished work on behalf of benckx May 16, 2026 09:09
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.

Add confirmation when importing move if there are already moves in the game

2 participants