#13435 - Android - Image annotation UX: text size not persisted, colo…

[Zeljko-Predjeskovic]

…r picker OK button on wrong side

#13429 - Svg editor color picker does not select correctly on cancelling or not selecting a color

[Zeljko-Predjeskovic]

only thing missing is how to switch ok and cancel button

[Zeljko-Predjeskovic]

[Zeljko-Predjeskovic]

also tested on android

[Zeljko-Predjeskovic]

clicking ok doesnt choose transparent. And cancelling keeps last picked color

[gentledepp]

only thing missing is how to switch ok and cancel button
should we then still merge the PR?

[Zeljko-Predjeskovic]

only thing missing is how to switch ok and cancel button
should we then still merge the PR?

for me its not possible to replace the content dialog quickly with a custom one. i can make another bug report for it. so yes i would merge it without the buttons in the correct position. The other issues seem more important.

[gentledepp]

Automated review pass focused on: software design choices, IDisposable usage, ReactiveUI/Rx correctness, Task await hygiene, and test coverage of the introduced changes.

Overall this is a solid change: it fixes a real pre-existing bug (an infinite re-prompt loop in TextInputService), and the new InputWithOptionsAsync/TextOptionsDialog* plumbing follows the existing dialog patterns in this codebase well. No ReactiveUI/Rx code is touched and no new IDisposable types were introduced. A few smaller points below.

[gentledepp]

Testing gap: no unit tests were added for this GetResult() or for ColorPickerDialogViewModel.GetResult()'s new transparent-color branch — both are pure logic and easily testable, unlike TextInputService which did get thorough new tests in this same PR.

[Zeljko-Predjeskovic]

i made the GetResult public in order to test it