[SBaksa]

Fixes #14302
Fixes #14279

What changes

• Define Pattern: OK or Enter with an empty or whitespace-only name keeps the dialog open, nothing is defined, and an alert titled "Pattern Name" says "You must specify a name for the pattern." Pressing OK again doesn't stack a second alert.
• Define Brush Preset: OK or Enter with an empty or whitespace-only name keeps the dialog open and creates nothing. No alert, and OK stays drawn enabled. A 300-character name is still accepted.
• Engine unchanged: programmatic calls (engine.execute, scripts, MCP) keep the default-name fallbacks ("Sampled Brush N", the pattern default name). Pre-filling the brush preset name in the dialog is BUG-DEFINE-BRUSH-PRESET-DEFAULT-NAME #14278.

How

• New dialogs/define_name.rs: a confirm preflight next to the existing fit_image_range / image_processor_ui ones. It only applies to the generic name dialog (__filter, no __spec) for these two commands.
• Error dialogs can carry a __title field (default stays "Error").

Tests

• crates/ui-egui/src/dialogs/define_pattern_empty_name_tests.rs: empty / missing / blank name alerts and keeps the dialog; repeated OK shows one alert; a named pattern still defines; programmatic default name kept.
• crates/ui-egui/src/define_brush_empty_name_tests.rs: empty / missing / blank name does nothing; named and 300-character names define; programmatic default name kept.
• With the preflight call commented out, the three empty-name tests fail and the rest pass.
• Checked end to end through the control channel: menu invoke, set a blank name, confirm twice. Both confirms return the alert message, the Define Pattern dialog stays open, and there is exactly one "Pattern Name" alert.
• fmt, clippy -D warnings and the wasm check of photocraft-ui-egui are clean.

Summary by CodeRabbit

• Bug Fixes
  • Pattern and brush preset dialogs now stay open when a name is missing or blank, and no preset or pattern is created.
  • Blank Pattern names show an error alert; blank brush preset names do not show an error dialog.
  • Error dialogs now display a supplied title when available, rather than always using “Error”.
  • Named patterns and brush presets, including those with long names, can still be confirmed.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Dialog confirmation now checks names for Define Pattern and Define Brush Preset commands. Blank names keep the dialog open. Pattern dialogs show a deduplicated error alert; brush preset dialogs do not show an alert. Tests cover named entries and programmatic default-name behavior.

Changes: Dialog name validation

Layer / File(s)	Summary
Name preflight and dialog confirmation crates/ui-egui/src/dialogs/define_name.rs, crates/ui-egui/src/dialogs.rs	Dialog confirmation runs a name preflight before closing. Blank pattern names return an error and show a deduplicated alert. Blank brush preset names return an empty error without an alert. Error dialog titles use the string-valued __title field when present, with "Error" as the fallback.
Pattern validation tests crates/ui-egui/src/dialogs/define_pattern_empty_name_tests.rs, crates/ui-egui/src/dialogs.rs	Tests cover missing, empty, and whitespace-only names; alert deduplication; named pattern creation; and the programmatic default-name fallback.
Brush preset validation tests crates/ui-egui/src/define_brush_empty_name_tests.rs, crates/ui-egui/src/lib.rs	Tests cover missing, empty, and whitespace-only names; named presets, including a 300-character name; and the programmatic Sampled Brush 1 fallback.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: davidmann4, trevorbrown-dev, wanikhawar

Merge Risk: Minimal · up to 66fb1

Define Pattern and Define Brush Preset now refuse empty or whitespace-only names while keeping the dialog open, and named entries still work. No actionable merge risk was found, and the change is small and easy to revert.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the two dialogs touched by the change (Define Pattern and Define Brush Preset) and the behavior they gain (refusing an empty name). The changeset adds a preflight check in define_name.…
Linked Issues check		[#14302, Define Pattern] The new preflight in crates/ui-egui/src/dialogs/define_name.rs handles the generic name dialog for edit.definePattern. A missing, non-string, empty, or whitespace-only `na…
Out of Scope Changes check		All changes support the two linked objectives. dialogs/define_name.rs holds the new preflight. dialogs.rs adds the __title fallback for Error dialogs and calls the preflight. lib.rs declares t…

Full details: Title check

Explanation

The title names the two dialogs touched by the change (Define Pattern and Define Brush Preset) and the behavior they gain (refusing an empty name). The changeset adds a preflight check in define_name.rs, wires it into dialogs.rs, and adds tests for empty, whitespace-only, and named inputs for both dialogs, so the title matches the main change.

Full details: Linked Issues check

Explanation

[#14302, Define Pattern] The new preflight in crates/ui-egui/src/dialogs/define_name.rs handles the generic name dialog for edit.definePattern. A missing, non-string, empty, or whitespace-only name opens an Error dialog with __title "Pattern Name" and the message "You must specify a name for the pattern.". It returns that message as an error, which keeps the dialog open. A repeated confirm does not add a second alert, because the code checks for an existing matching Error dialog. A non-empty name passes through unchanged. The engine default-name fallback is not changed, so edit.definePattern without name keeps its default. The __title field in dialogs.rs supplies the alert title and keeps "Error" as the default. Tests in define_pattern_empty_name_tests.rs cover these cases, as the PR description reports.

[#14279, Define Brush Preset] For edit.defineBrushPreset, the same preflight returns Err(String::new()) for a blank name. This blocks the define without an alert, so the OK button is not changed. A 300-character name is not blank and passes. The engine define_brush_preset code is not changed, so the programmatic default "Sampled Brush N" is kept. Tests in define_brush_empty_name_tests.rs cover these cases.

Not verified: I did not read the caller of preflight in dialogs.rs. The PASS for the Enter path and for the handling of an empty error string depends on the PR's reported end-to-end check and tests, not on code I read directly.

Full details: Out of Scope Changes check

Explanation

All changes support the two linked objectives. dialogs/define_name.rs holds the new preflight. dialogs.rs adds the __title fallback for Error dialogs and calls the preflight. lib.rs declares the test modules. The two test files cover the stated cases. The __title change keeps the default "Error" title, so existing error dialogs do not change. No parity docs were edited, and no engine files were changed.

Note on the issue "Where" lists: #14302 names dialogs/filter_dialog.rs and #14279 names prefs_ui/dialogs.rs as the change points. This PR does not touch either file. It places the logic in a new module and the shared dialogs.rs. This is a deviation from the issue process rules, not an unrelated change. It does not affect the behavior assessed above.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/photocraft
• Branch: main
• File: .coderabbit.yaml
• Commit: 5b27ff6
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 470240de-b3fb-4b69-a78d-a325f07b5aa4

Commits

Reviewing files that changed from the base of the PR and between 5b27ff6 and 66fb150.

Files selected for processing: 5

• crates/ui-egui/src/define_brush_empty_name_tests.rs
• crates/ui-egui/src/dialogs.rs
• crates/ui-egui/src/dialogs/define_name.rs
• crates/ui-egui/src/dialogs/define_pattern_empty_name_tests.rs
• crates/ui-egui/src/lib.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.