Fix the folder picker stuck on "Loading…"
The picker set loading=true and nothing ever cleared it, because browser_loaded() was never called — my earlier edit to replace the stub silently failed to apply after cargo fmt reindented the code it matched against. The stale stub was still writing folder names into the status line, which is the list visible under the selector. Verified by grep before and after: browser_loaded had zero call sites, now two. Two related defects fixed while in there. Both timers polled their channel with `if let Ok(..)`, so a worker thread that died without sending left the screen on "Loading…" or "Connecting…" forever. Both now treat Disconnected as terminal. The first attempt at that fix introduced a bug of its own: a separate probe call consumed a pending message and discarded it, so a successful login would have vanished. The login poller now drains in one loop with disconnection as a match arm, and only reports failure when the flow had not already completed. Lesson worth recording: verify a replacement applied rather than trusting the edit succeeded. A silently-skipped replace looks identical to a successful one until the feature is used. 43 tests in dr-ui.
This commit is contained in:
+62
-16
@@ -339,7 +339,29 @@ fn poll_channel(
|
||||
move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let ctl = &ctl_for_cb;
|
||||
while let Ok(msg) = rx.try_recv() {
|
||||
// Drain in one loop. A separate probe call would consume a
|
||||
// pending message and throw it away — a successful login would
|
||||
// vanish. Disconnection is handled as a terminal case here, so a
|
||||
// worker that dies without sending cannot leave the screen on
|
||||
// "Connecting…" forever.
|
||||
loop {
|
||||
let msg = match rx.try_recv() {
|
||||
Ok(m) => m,
|
||||
Err(std::sync::mpsc::TryRecvError::Empty) => break,
|
||||
Err(std::sync::mpsc::TryRecvError::Disconnected) => {
|
||||
// Only an error if the flow never completed; a
|
||||
// successful login stops the timer below.
|
||||
if !ctl.model.borrow().is_signed_in() {
|
||||
ctl.model.borrow_mut().fail("sign-in failed unexpectedly");
|
||||
render(&w, ctl);
|
||||
}
|
||||
if let Some(t) = ctl.poll_timer.borrow().as_ref() {
|
||||
t.stop();
|
||||
}
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let mut done = false;
|
||||
match msg {
|
||||
LoginMessage::AwaitingApproval(url) => {
|
||||
@@ -432,24 +454,48 @@ fn spawn_folder_list(weak: slint::Weak<AppWindow>, ctl: Rc<LaunchController>, pa
|
||||
move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let ctl = &ctl_for_cb;
|
||||
|
||||
// One try_recv covers both outcomes. A panicking worker drops
|
||||
// the sender without sending; treating that as "still loading"
|
||||
// is exactly what leaves the picker stuck forever.
|
||||
let outcome = match rx.try_recv() {
|
||||
Ok(result) => Some(result),
|
||||
Err(std::sync::mpsc::TryRecvError::Empty) => None,
|
||||
Err(std::sync::mpsc::TryRecvError::Disconnected) => {
|
||||
Some(Err("folder listing failed unexpectedly".to_string()))
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(result) = outcome {
|
||||
match result {
|
||||
// Hand the listing to the model, which clears `loading`
|
||||
// and populates the picker.
|
||||
Ok(dirs) => ctl.model.borrow_mut().browser_loaded(dirs),
|
||||
Err(e) => {
|
||||
// Close the picker before reporting: leaving it open
|
||||
// on a permanent "Loading…" is what this replaced.
|
||||
ctl.model.borrow_mut().close_browser();
|
||||
ctl.model.borrow_mut().fail(e);
|
||||
}
|
||||
}
|
||||
render(&w, ctl);
|
||||
if let Some(t) = ctl.poll_timer.borrow().as_ref() {
|
||||
t.stop();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if let Ok(result) = rx.try_recv() {
|
||||
match result {
|
||||
Ok(dirs) => {
|
||||
// No folder picker yet: report what is there and
|
||||
// leave selection to the CLI, rather than
|
||||
// pretending to offer a chooser.
|
||||
let msg = if dirs.is_empty() {
|
||||
"No folders found.".to_string()
|
||||
} else {
|
||||
format!("Folders: {}", dirs.join(", "))
|
||||
};
|
||||
let session = ctl.model.borrow().session().cloned();
|
||||
if let Some(s) = session {
|
||||
ctl.model.borrow_mut().signed_in(s);
|
||||
}
|
||||
ctl.model.borrow_mut().status = Some(msg);
|
||||
// Hand the listing to the model, which clears `loading`
|
||||
// and populates the picker.
|
||||
Ok(dirs) => ctl.model.borrow_mut().browser_loaded(dirs),
|
||||
Err(e) => {
|
||||
// Close the picker before reporting: leaving it open
|
||||
// on a permanent "Loading…" is what this replaced.
|
||||
ctl.model.borrow_mut().close_browser();
|
||||
ctl.model.borrow_mut().fail(e);
|
||||
}
|
||||
Err(e) => ctl.model.borrow_mut().fail(e),
|
||||
}
|
||||
render(&w, ctl);
|
||||
if let Some(t) = ctl.poll_timer.borrow().as_ref() {
|
||||
|
||||
Reference in New Issue
Block a user