Skip to content

Commit 8ea84b0

Browse files
committed
refactor: Use Query debounce for create dialog validation
- Replace manual debouncing with Query::debounce() for prefill suggestions (300ms) - Add Query-based URL validation with 500ms debounce and 10s timeout - Remove prefill_generation counter (eliminates race conditions) - Add live validation feedback as users type - Improve error handling with visual feedback and toasts - Maintain Flatpak compatibility via CommandRunner
1 parent cbebeb7 commit 8ea84b0

1 file changed

Lines changed: 127 additions & 88 deletions

File tree

src/dialogs/create_distrobox_dialog.rs

Lines changed: 127 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ use adw::subclass::prelude::*;
33
use gtk::gio::File;
44
use gtk::{gio, glib};
55
use std::cell::Cell;
6+
use std::time::Duration;
67
use tracing::error;
78

89
use crate::backends::{self, CreateArgName, CreateArgs, Error};
910
use crate::dialogs::create_distrobox_helpers::split_repo_tag_digest;
1011
use crate::fakers::Command;
1112
use crate::i18n::gettext;
1213
use crate::models::Container;
14+
use crate::query::Query;
1315
use crate::root_store::RootStore;
1416
use crate::widgets::{ImageRowItem, SidebarRow};
1517

@@ -42,7 +44,8 @@ mod imp {
4244
pub image_row: adw::ActionRow,
4345
pub images_model: gtk::StringList,
4446
pub selected_image: RefCell<String>,
45-
pub prefill_generation: std::cell::Cell<u64>,
47+
pub prefill_query: RefCell<Option<Query<Option<String>>>>,
48+
pub url_validation_query: RefCell<Option<Query<bool>>>,
4649
pub home_row_expander: adw::ExpanderRow,
4750
#[property(get, set, nullable)]
4851
pub home_folder: RefCell<Option<String>>,
@@ -339,61 +342,70 @@ impl CreateDistroboxDialog {
339342
));
340343

341344
// Prefill wiring: debounce name changes to suggest an image when user hasn't interacted
342-
imp.name_row.connect_changed(clone!(
345+
let prefill_query: Query<Option<String>> = Query::new(
346+
"prefill-suggestions".to_string(),
347+
clone!(
348+
#[weak(rename_to=this)]
349+
self,
350+
#[upgrade_or_panic]
351+
move || async move {
352+
let imp = this.imp();
353+
let text = imp.name_row.text().to_string();
354+
355+
// don't prefill if cloning from a source
356+
if imp.clone_src.borrow().is_some() {
357+
return Ok(None);
358+
}
359+
360+
if text.is_empty() {
361+
if imp.selected_image.borrow().is_empty() {
362+
return Ok(Some(gettext("Select an image...")));
363+
}
364+
return Ok(None);
365+
}
366+
367+
let candidates = imp
368+
.images_model
369+
.snapshot()
370+
.into_iter()
371+
.filter_map(|item| {
372+
item.downcast::<gtk::StringObject>()
373+
.ok()
374+
.map(|sobj| sobj.string().to_string())
375+
})
376+
.collect::<Vec<_>>();
377+
378+
let (_filter, suggested_opt) =
379+
crate::dialogs::create_distrobox_helpers::derive_image_prefill(
380+
&text,
381+
Some(&candidates),
382+
);
383+
384+
Ok(suggested_opt)
385+
}
386+
),
387+
);
388+
389+
prefill_query.connect_success(clone!(
343390
#[weak(rename_to=this)]
344391
self,
345-
move |entry| {
392+
move |suggested_opt| {
346393
let imp = this.imp();
347-
let prefill_gen = imp.prefill_generation.get().wrapping_add(1);
348-
imp.prefill_generation.set(prefill_gen);
349-
let text = entry.text().to_string();
350-
let obj_inner = this.clone();
351-
glib::MainContext::ref_thread_default().spawn_local(clone!(
352-
#[weak]
353-
obj_inner,
354-
async move {
355-
glib::timeout_future(std::time::Duration::from_millis(300)).await;
356-
let imp = obj_inner.imp();
357-
if imp.prefill_generation.get() != prefill_gen {
358-
return;
359-
}
360-
// don't prefill if cloning from a source
361-
if imp.clone_src.borrow().is_some() {
362-
return;
363-
}
364-
if text.is_empty() {
365-
if imp.selected_image.borrow().is_empty() {
366-
imp.image_row.set_subtitle(&gettext("Select an image..."));
367-
}
368-
} else {
369-
let candidates = imp
370-
.images_model
371-
.snapshot()
372-
.into_iter()
373-
.filter_map(|item| {
374-
item.downcast::<gtk::StringObject>()
375-
.ok()
376-
.map(|sobj| sobj.string().to_string())
377-
})
378-
.collect::<Vec<_>>();
379-
380-
let (_filter, suggested_opt) =
381-
crate::dialogs::create_distrobox_helpers::derive_image_prefill(
382-
&text,
383-
Some(&candidates),
384-
);
385-
if let Some(suggested) = suggested_opt {
386-
// set subtitle as tentative prefill (do not overwrite confirmed selection)
387-
if imp.selected_image.borrow().is_empty() {
388-
imp.image_row.set_subtitle(&suggested);
389-
}
390-
}
391-
}
394+
if let Some(suggested) = suggested_opt.as_ref() {
395+
// set subtitle as tentative prefill (do not overwrite confirmed selection)
396+
if imp.selected_image.borrow().is_empty() {
397+
imp.image_row.set_subtitle(suggested);
392398
}
393-
));
399+
}
394400
}
395401
));
396402

403+
*imp.prefill_query.borrow_mut() = Some(prefill_query.clone());
404+
405+
imp.name_row.connect_changed(move |_| {
406+
prefill_query.refetch_with(Query::debounce(Duration::from_millis(300)));
407+
});
408+
397409
page
398410
}
399411
pub fn build_assemble_from_file_page(&self) -> adw::NavigationPage {
@@ -474,56 +486,85 @@ impl CreateDistroboxDialog {
474486
create_btn.set_sensitive(false);
475487
content.append(&create_btn);
476488

477-
url_row.connect_changed(clone!(
489+
// Create URL validation query with debouncing
490+
let url_validation_query: Query<bool> = Query::new(
491+
"url-validation".to_string(),
492+
clone!(
493+
#[weak(rename_to=this)]
494+
self,
495+
#[upgrade_or_panic]
496+
move || async move {
497+
if let Some(url_text) = this.assemble_url() {
498+
if url_text.is_empty() {
499+
return Ok(false);
500+
}
501+
this.validate_url(&url_text).await
502+
} else {
503+
Ok(false)
504+
}
505+
}
506+
),
507+
).with_timeout(Duration::from_secs(10));
508+
509+
url_validation_query.connect_success(clone!(
478510
#[weak(rename_to=this)]
479511
self,
480512
#[weak]
481513
create_btn,
482-
move |entry| {
483-
this.set_assemble_url(Some(entry.text()));
484-
this.set_url_validated(false);
514+
#[weak]
515+
url_row,
516+
move |is_valid| {
517+
this.set_url_validated(*is_valid);
518+
create_btn.set_sensitive(*is_valid);
519+
520+
if !is_valid {
521+
let toast = adw::Toast::new(&gettext("Could not connect to URL"));
522+
this.imp().toast_overlay.add_toast(toast);
523+
url_row.add_css_class("error");
524+
} else {
525+
url_row.remove_css_class("error");
526+
}
527+
}
528+
));
529+
530+
url_validation_query.connect_error(clone!(
531+
#[weak]
532+
create_btn,
533+
#[weak]
534+
url_row,
535+
move |_| {
485536
create_btn.set_sensitive(false);
486-
// Clear error CSS when user types
487-
entry.remove_css_class("error");
537+
url_row.add_css_class("error");
488538
}
489539
));
490540

491-
url_row.connect_apply(clone!(
541+
*self.imp().url_validation_query.borrow_mut() = Some(url_validation_query.clone());
542+
543+
url_row.connect_changed(clone!(
492544
#[weak(rename_to=this)]
493545
self,
494546
#[weak]
495547
create_btn,
548+
#[strong]
549+
url_validation_query,
496550
move |entry| {
497-
let url = entry.text().to_string();
498-
if url.is_empty() {
499-
return;
500-
}
501-
502-
// Reset validation state
551+
this.set_assemble_url(Some(entry.text()));
503552
this.set_url_validated(false);
504553
create_btn.set_sensitive(false);
554+
// Clear error CSS when user types
555+
entry.remove_css_class("error");
556+
557+
// Debounced validation
558+
url_validation_query.refetch_with(Query::debounce(Duration::from_millis(500)));
559+
}
560+
));
505561

506-
glib::MainContext::ref_thread_default().spawn_local(clone!(
507-
#[weak]
508-
this,
509-
#[weak]
510-
create_btn,
511-
#[weak]
512-
entry,
513-
async move {
514-
let is_valid = this.validate_url(&url).await;
515-
this.set_url_validated(is_valid);
516-
create_btn.set_sensitive(is_valid);
517-
518-
if !is_valid {
519-
let toast = adw::Toast::new(&gettext("Could not connect to URL"));
520-
this.imp().toast_overlay.add_toast(toast);
521-
entry.add_css_class("error");
522-
} else {
523-
entry.remove_css_class("error");
524-
}
525-
}
526-
));
562+
// Also validate immediately on Apply button press
563+
url_row.connect_apply(clone!(
564+
#[strong]
565+
url_validation_query,
566+
move |_| {
567+
url_validation_query.refetch_with(Query::immediate());
527568
}
528569
));
529570

@@ -947,7 +988,7 @@ impl CreateDistroboxDialog {
947988
}
948989
}
949990

950-
async fn validate_url(&self, url: &str) -> bool {
991+
async fn validate_url(&self, url: &str) -> anyhow::Result<bool> {
951992
// Use curl with HEAD request to validate URL
952993
// CRITICAL: Use self.root_store().command_runner() for Flatpak compatibility
953994
let command_runner = self.root_store().command_runner();
@@ -959,9 +1000,7 @@ impl CreateDistroboxDialog {
9591000
cmd.arg("5"); // 5 second connection timeout
9601001
cmd.arg(url);
9611002

962-
match command_runner.output(cmd).await {
963-
Ok(output) => output.status.success(),
964-
Err(_) => false,
965-
}
1003+
let output = command_runner.output(cmd).await?;
1004+
Ok(output.status.success())
9661005
}
9671006
}

0 commit comments

Comments
 (0)