Skip to content

Reset confirmation dialog layout after dismissal#684

Draft
righteousgambit wants to merge 1 commit into
Carvera-Community:developfrom
righteousgambit:agent/reset-confirm-popup-layout
Draft

Reset confirmation dialog layout after dismissal#684
righteousgambit wants to merge 1 commit into
Carvera-Community:developfrom
righteousgambit:agent/reset-confirm-popup-layout

Conversation

@righteousgambit

Copy link
Copy Markdown

Closes #683

Summary

  • restore the reusable confirmation popup's standard size, position, and title sizing after dismissal
  • keep the intentionally expanded laser-mode and resume-at-line warning layouts unchanged while they are open
  • add hardware-free integration regressions for both expanded-warning -> ordinary-confirmation sequences
  • document the fix in the unreleased changelog

This is deliberately limited to presentation state. It does not change confirm/cancel callbacks, controller commands, modal dismissal policy, or the content of either safety warning.

Root cause

Makera owns one ConfirmPopup for all confirmation workflows. The standard geometry is applied by KV when that singleton is constructed. Laser mode later mutates its size, position, and title height; resume-at-line mutates its size and position. Before this change, on_dismiss() only cleared the showing flag, so those live properties became the next workflow's starting state.

The reset belongs in the popup dismissal lifecycle so every expanded workflow is contained without requiring every ordinary caller to defensively re-declare the defaults.

Deterministic reproduction and proof

Validated from pinned upstream develop@2ea66c10e96d88013a04441134d3d4f8e8c88510 with the repository's locked Python 3.11 environment.

The regression opens an expanded warning, verifies its intended layout, dismisses it, opens Restore Settings, and verifies the standard layout. It covers both:

  • Laser mode: (0.6, 0.7), centered, automatic title height -> standard confirmation
  • Resume-at-line: (0.8, 0.8), centered -> standard confirmation

On the pinned base, the first regression failed deterministically:

E       assert (0.6, 0.7) == (0.5, 0.4)
1 failed

With this change:

poetry run python -m pytest -q tests/integration/test_confirm_popup.py
2 passed, 1 warning

The warning is the repository's existing Python deprecation warning for locale.getdefaultlocale().

Full validation

Real fork commit: a61390716db8

Its sole parent is pinned upstream develop@2ea66c10e96d88013a04441134d3d4f8e8c88510.

poetry run python -m pytest -q
22 passed, 15 skipped, 1 warning in 42.35s

ruff check carveracontroller/main.py tests/integration/test_confirm_popup.py
All checks passed!

ruff format --check carveracontroller/main.py tests/integration/test_confirm_popup.py
2 files already formatted

git diff --check
passed

All 15 skipped cases are the repository's existing visual-regression tests, which call pytest.skip() when their reference PNG is absent; there are currently no checked-in reference images. The new regressions therefore assert exact Kivy state and geometry and do not rely on those non-enforcing visual cases.

Operator-facing result

After an operator cancels an expanded laser or resume warning, short confirmations such as Restore Settings, Save As Default, delete, overwrite, and reset again use the normal dialog hierarchy and footprint. The expanded safety prompts themselves retain their current prominence.

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.

1 participant