Replace the six view booleans with View and Page enums
app.slint carried show-launch, show-library, show-identity, show-settings,
show-import and show-merge as separate booleans, so the root component chose
what to draw with five- and six-term conjunctions and nothing stopped two of
them being true at once. Replaced with two enums: View { develop, library,
identity, launch } for which top-level screen is showing, and Page { none,
settings, import, merge } for which page, if any, is drawn over it.
Two values rather than one, because the two questions are genuinely
different. Settings, Import and Merge are reachable from more than one View
and are drawn outermost without touching it — closing one has to return to
whichever View was already current, and today that works because the
underlying property is left alone while the page sits over it. A single
View with five or more variants would need a second field remembering what
to return to; Page needs nothing to remember, since View was never
overwritten in the first place. Identity, by contrast, genuinely replaces
the window the way Launch and Library do (see the existing "like the launch
screen" comment on its `if`), so it is a View variant, not a Page.
Every `if` chain in app.slint that used to compare four, five or six
booleans now compares active-view and active-page to at most one variant
each. library-visible collapsed from a six-term conjunction to
`active-page == Page.none && active-view == View.library`.
The Rust side follows: every set_show_*/get_show_* call in library_ui.rs,
identity_ui.rs, settings_ui.rs, merge_ui.rs, import_ui.rs, launch_ui.rs and
lib.rs now reads or writes active-view or active-page instead, including
lib.rs's startup match (View.launch vs View.develop, since a Startup that
skips the launch screen used to leave both old booleans false and fall
through the chain to develop) and identity_ui's close handler, which now
writes View.library or View.develop in one call where it used to write
show-library then show-identity separately.
back_one_step needed one deliberate adjustment beyond the mechanical
rename. Identity was never represented in NavState: back had nothing to do
when Identity was opened from the library (show-library stayed true,
unread by IdentityScreen's own condition) and could only reach ToLibrary
when opened from develop, which likewise wrote a property IdentityScreen
never read — so escaping out of Identity was invisible in both cases before
this change. With a single active-view, falling into the general case
would instead overwrite the value IdentityScreen's `if` does read and close
it as an unintended side effect. back_one_step now swallows the gesture
while View.identity is current, reproducing the same "nothing visible
happens" outcome for both origins without threading identity_ui's private
came-from-library state through lib.rs for one screen.
Verified with tools/manual/drive.py against a private Xvfb and the debug
build: launch screen to library, Settings opened and closed, Identity
opened and closed (including Escape doing nothing while it is open),
develop opened from a cell and closed both by the back button and by
Escape. Screenshots under verify/.
This commit is contained in:
@@ -17,7 +17,7 @@ use slint::{ComponentHandle, Model as _, ModelRc, VecModel};
|
||||
|
||||
use crate::faces::FaceSweepMessage;
|
||||
use crate::identity::{self, FaceCell, PersonRow};
|
||||
use crate::{AppWindow, IdentityFace, IdentityPerson};
|
||||
use crate::{AppWindow, IdentityFace, IdentityPerson, View};
|
||||
|
||||
/// Which model's faces the screen is looking at.
|
||||
///
|
||||
@@ -740,14 +740,14 @@ fn wire_navigation(
|
||||
let models_present = models.clone();
|
||||
window.on_identity_open(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let from_library = w.get_show_library();
|
||||
let from_library = w.get_active_view() == View::Library;
|
||||
ctl.came_from_library.set(from_library);
|
||||
w.set_identity_back_label(if from_library {
|
||||
"‹ Library".into()
|
||||
} else {
|
||||
"‹ Develop".into()
|
||||
});
|
||||
w.set_show_identity(true);
|
||||
w.set_active_view(View::Identity);
|
||||
// A fact about the filesystem, so it is re-checked on every open
|
||||
// rather than cached: the user may have just put the models there.
|
||||
w.set_identity_model_missing(models_present().is_none());
|
||||
@@ -768,8 +768,11 @@ fn wire_navigation(
|
||||
// Back to whichever screen this was opened from. The develop
|
||||
// session was never torn down — it was only hidden — so returning
|
||||
// to it costs nothing and keeps the photographer's place.
|
||||
w.set_show_library(ctl.came_from_library.get());
|
||||
w.set_show_identity(false);
|
||||
w.set_active_view(if ctl.came_from_library.get() {
|
||||
View::Library
|
||||
} else {
|
||||
View::Develop
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ use dr_types::{FormatFilter, ImportSettings};
|
||||
|
||||
use crate::activity::{Activity, ActivityLog, Kind};
|
||||
use crate::import::{self, Message, Request, Upload};
|
||||
use crate::AppWindow;
|
||||
use crate::{AppWindow, Page};
|
||||
|
||||
/// How often the drain runs while an import is going.
|
||||
///
|
||||
@@ -344,7 +344,7 @@ where
|
||||
*ctl.error.borrow_mut() = String::new();
|
||||
survey(&w, &ctl, &context);
|
||||
render(&w, &ctl);
|
||||
w.set_show_import(true);
|
||||
w.set_active_page(Page::Import);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -358,7 +358,7 @@ where
|
||||
if ctl.running.get() {
|
||||
return;
|
||||
}
|
||||
w.set_show_import(false);
|
||||
w.set_active_page(Page::None);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ use dr_sync_nextcloud::{auth, NextcloudProvider};
|
||||
use slint::ComponentHandle;
|
||||
|
||||
use crate::launch::{LaunchModel, LaunchState};
|
||||
use crate::AppWindow;
|
||||
use crate::{AppWindow, View};
|
||||
|
||||
/// Shared launch state for the running window.
|
||||
pub struct LaunchController {
|
||||
@@ -257,7 +257,7 @@ fn wire_choose_folder_and_open<F>(
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let session = ctl.model.borrow().session().cloned();
|
||||
if let Some(s) = session {
|
||||
w.set_show_launch(false);
|
||||
w.set_active_view(View::Library);
|
||||
on_open_library(s);
|
||||
}
|
||||
});
|
||||
|
||||
+26
-4
@@ -1618,7 +1618,13 @@ fn construct_screens(
|
||||
{
|
||||
let controller = launch_ui::LaunchController::new();
|
||||
let startup = controller.model.borrow().startup_action(!paths.is_empty());
|
||||
window.set_show_launch(startup == launch::Startup::ShowLaunchScreen);
|
||||
// `OpenLibrary` is overwritten moments later by `library_ui::open`
|
||||
// below, once a stored session exists; `ShowLocalFiles` leaves
|
||||
// `develop` as it found it, since there is no library to switch to.
|
||||
window.set_active_view(match startup {
|
||||
launch::Startup::ShowLaunchScreen => View::Launch,
|
||||
launch::Startup::ShowLocalFiles | launch::Startup::OpenLibrary => View::Develop,
|
||||
});
|
||||
|
||||
let library = library.clone();
|
||||
let collections = collections.clone();
|
||||
@@ -3404,11 +3410,27 @@ fn apply_layout_class(
|
||||
/// the activity — the behaviour every application there has, and the reason
|
||||
/// this answers with a bool rather than swallowing the gesture.
|
||||
fn back_one_step(w: &AppWindow) -> bool {
|
||||
// The Identity screen has no representation in `NavState`: nothing here
|
||||
// records which view it replaced, only `identity_ui`'s own
|
||||
// `came_from_library`, which this function has no reason to reach for
|
||||
// over one screen. Before this refactor, back queued nothing behind
|
||||
// Identity opened from the library (show-library stayed true, unread by
|
||||
// anything) and queued `ToLibrary` behind Identity opened from develop —
|
||||
// but `ToLibrary` only ever set `show-library`, a property
|
||||
// `IdentityScreen`'s own `if` never read, so both cases looked the same
|
||||
// on screen: nothing happened. Swallowing the gesture here keeps that:
|
||||
// falling into the general case below would instead write `active-view`,
|
||||
// which `IdentityScreen`'s `if` *does* read, and close the screen as a
|
||||
// side effect nobody asked for.
|
||||
if w.get_active_view() == View::Identity {
|
||||
return true;
|
||||
}
|
||||
|
||||
let state = NavState {
|
||||
settings: w.get_show_settings(),
|
||||
launch: w.get_show_launch(),
|
||||
settings: w.get_active_page() == Page::Settings,
|
||||
launch: w.get_active_view() == View::Launch,
|
||||
browsing: w.get_launch_browsing(),
|
||||
library: w.get_show_library(),
|
||||
library: w.get_active_view() == View::Library,
|
||||
mode: w.global::<Develop>().get_view_mode(),
|
||||
zoomed: w.get_zoomed(),
|
||||
// Files named on the command line have no grid behind them — the same
|
||||
|
||||
+14
-14
@@ -22,7 +22,7 @@ use dr_types::FormatFilter;
|
||||
use slint::{ComponentHandle, Model as _};
|
||||
|
||||
use crate::library::{self, ScanMessage, ThumbnailMessage};
|
||||
use crate::{AppWindow, GestureRow, KeywordRow, LibraryCell, PersonChip, TimelineBar};
|
||||
use crate::{AppWindow, GestureRow, KeywordRow, LibraryCell, PersonChip, TimelineBar, View};
|
||||
|
||||
/// A screenful before the grid has reported its geometry.
|
||||
///
|
||||
@@ -957,7 +957,7 @@ pub fn open(
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
window.set_library_error(format!("credentials: {e}").into());
|
||||
window.set_show_library(true);
|
||||
window.set_active_view(View::Library);
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -977,7 +977,7 @@ pub fn open(
|
||||
// another device may still be applied — see `place_untouched`.
|
||||
ctl.place_untouched.set(true);
|
||||
|
||||
window.set_show_library(true);
|
||||
window.set_active_view(View::Library);
|
||||
window.set_library_open(true);
|
||||
window.set_library_scanning(true);
|
||||
window.set_library_error(slint::SharedString::new());
|
||||
@@ -5391,7 +5391,7 @@ fn write_place(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
fn current_place(window: &AppWindow, ctl: &Rc<LibraryController>) -> Option<dr_types::Place> {
|
||||
use dr_types::{Place, PlaceScope, Screen};
|
||||
|
||||
if window.get_show_launch() || !window.get_library_open() {
|
||||
if window.get_active_view() == View::Launch || !window.get_library_open() {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -5401,7 +5401,7 @@ fn current_place(window: &AppWindow, ctl: &Rc<LibraryController>) -> Option<dr_t
|
||||
// tracks; in develop it is the open one, which `report_position` keeps
|
||||
// `index` holding. Both are library ordinals against the same ordering, so
|
||||
// the lookup below is one piece of code rather than two.
|
||||
let in_library = window.get_show_library();
|
||||
let in_library = window.get_active_view() == View::Library;
|
||||
let at = if in_library {
|
||||
ctl.resume_at.get()
|
||||
} else {
|
||||
@@ -5544,7 +5544,7 @@ pub(crate) fn apply_place(
|
||||
if place.screen == Screen::Develop {
|
||||
if let Some((at, Found::Photograph)) = resolved {
|
||||
if let Some(open) = ctl.open_image.borrow().clone() {
|
||||
window.set_show_library(false);
|
||||
window.set_active_view(View::Develop);
|
||||
window.set_library_roll_centre(true);
|
||||
let offset = *ctl.offset.borrow();
|
||||
if let Some(row) = at.checked_sub(offset) {
|
||||
@@ -5836,7 +5836,7 @@ pub fn wire<F>(
|
||||
if let Some(path) = path {
|
||||
// Leave the grid for the develop view. The status bar's
|
||||
// "‹ Library" button comes back here.
|
||||
w.set_show_library(false);
|
||||
w.set_active_view(View::Develop);
|
||||
// Which cell the develop view is now showing, so the photo
|
||||
// roll opens marking it rather than marking nothing.
|
||||
w.set_library_roll_current(i);
|
||||
@@ -5940,7 +5940,7 @@ fn wire_grid_cursor_and_zoom(
|
||||
let row = cursor.checked_sub(offset);
|
||||
let path = row.and_then(|row| ctl.paths.borrow().get(row).cloned());
|
||||
if let Some(path) = path {
|
||||
w.set_show_library(false);
|
||||
w.set_active_view(View::Develop);
|
||||
// As on a click: the roll marks what is open, and centres on it
|
||||
// because this too begins a session.
|
||||
w.set_library_roll_current(row.unwrap_or(0) as i32);
|
||||
@@ -6367,7 +6367,7 @@ fn wire_grid_routes(
|
||||
let weak = window.as_weak();
|
||||
window.on_library_change(move || {
|
||||
if let Some(w) = weak.upgrade() {
|
||||
w.set_show_launch(true);
|
||||
w.set_active_view(View::Launch);
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -6401,10 +6401,11 @@ fn wire_grid_routes(
|
||||
let open = w.get_index().max(0) as usize;
|
||||
resume_position(&w, &ctl, &coll_ctl, Some(open));
|
||||
|
||||
w.set_show_library(true);
|
||||
w.set_active_view(View::Library);
|
||||
// TRACES: FR-UI-8
|
||||
// After the flag, not before: `current_place` reads it to say which
|
||||
// view the record is of, and this is the moment it becomes the grid.
|
||||
// After the view is set, not before: `current_place` reads it to
|
||||
// say which view the record is of, and this is the moment it
|
||||
// becomes the grid.
|
||||
write_place(&w, &ctl);
|
||||
});
|
||||
}
|
||||
@@ -6640,8 +6641,7 @@ fn wire_filter_ratings_and_people(window: &AppWindow, ctl: &Rc<LibraryController
|
||||
// Leaving the Identity screen for the grid is the whole point of
|
||||
// the button: the answer to "who is this" is a set of photographs,
|
||||
// and they are shown where photographs are shown.
|
||||
w.set_show_identity(false);
|
||||
w.set_show_library(true);
|
||||
w.set_active_view(View::Library);
|
||||
refilter(&w, &ctl);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@ use crate::merge::{
|
||||
self, AlignmentReport, Cancel, Decision, FillSettings, MergeDestination, MergeEvent,
|
||||
MergeInput, MergeRequest,
|
||||
};
|
||||
use crate::{AppWindow, MergeFrameRow};
|
||||
use crate::{AppWindow, MergeFrameRow, Page};
|
||||
|
||||
/// How often the page reads the job's channel.
|
||||
const DRAIN_INTERVAL: std::time::Duration = std::time::Duration::from_millis(120);
|
||||
@@ -411,7 +411,7 @@ fn wire_stop_and_leave(window: &AppWindow, ctl: &Rc<MergeController>) {
|
||||
job.activity.finish_quietly();
|
||||
}
|
||||
*ctl.timer.borrow_mut() = None;
|
||||
w.set_show_merge(false);
|
||||
w.set_active_page(Page::None);
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -481,7 +481,7 @@ fn start<Fetch>(
|
||||
window.set_merge_fill_note("".into());
|
||||
window.set_merge_preview_filled(false);
|
||||
window.set_merge_done(false);
|
||||
window.set_show_merge(true);
|
||||
window.set_active_page(Page::Merge);
|
||||
|
||||
let timer = slint::Timer::default();
|
||||
{
|
||||
|
||||
@@ -32,7 +32,7 @@ use dr_types::{
|
||||
use slint::ComponentHandle;
|
||||
|
||||
use crate::settings_store::SettingsStore;
|
||||
use crate::{Adjustments, AppWindow};
|
||||
use crate::{Adjustments, AppWindow, Page};
|
||||
use dr_sync::Connection;
|
||||
|
||||
/// Shared settings state for the running window.
|
||||
@@ -436,7 +436,7 @@ fn wire_open_close(
|
||||
*ctl.settings.borrow_mut() = ctl.store.load();
|
||||
render(&w, &ctl);
|
||||
on_open(&w);
|
||||
w.set_show_settings(true);
|
||||
w.set_active_page(Page::Settings);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -444,7 +444,7 @@ fn wire_open_close(
|
||||
let weak = window.as_weak();
|
||||
window.on_settings_close(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
w.set_show_settings(false);
|
||||
w.set_active_page(Page::None);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user