Fit the interface to the system bars, the finger and the back key
Four faults that only show on a device, and one that was hiding on the desktop too. The system bars. Target SDK 36 forces edge-to-edge, so the window spans the display and the develop status strip was drawn underneath the clock and the wifi icons. Slint already computes the inset from Android's OnApplyWindowInsetsListener and exposes it as Window.safe-area-insets; nothing read it. The four views now sit inside a shell placed within the safe area. Every inset is zero on the desktop, so that layout does not move. Sliders under a finger. A Flickable steals any gesture that drifts more than 8 logical pixels along its scrolling axis within half a second of the press, and it steals it by cancelling the child. ParamSlider's axis test correctly declined to claim vertical drags, but nothing told the Flickable to stand down once a drag was claimed — so an adjustment would start moving and then be taken away mid-motion. A mouse holds a horizontal line closely enough to stay under 8px; a finger does not, which is why these worked on the desktop and not on the tablet. The claim now sets `interactive: false` for the rest of the gesture. The tone curve had the same fault and worse: its points are dragged vertically, which is the Flickable's own axis, so every drag was stolen — on the desktop as well. The back gesture. Nothing handled it, so back closed the application from anywhere in it. Android delivers it as Key.Back to the focused item and bubbles it up the ancestors, which is the second reason the shell wraps the views rather than sitting beside them. The order is innermost first: settings, then crop, then zoom, then develop to the grid, then a collection scope. Answering false at the top of the stack leaves Android to close the activity, as it does for every other application there. Escape does the same on a keyboard. back_step is a pure function over a flat NavState so the ordering can be tested without a backend: which of two states is left first is the whole of the feature, and it is the part that is easy to get subtly wrong when spelled out in nested ifs over live properties. Develop's canvas now takes focus on show. Without it the arrow keys did nothing until the canvas was clicked, and Key.Back had no focus item to bubble from. Panels that close. The collections sidebar and the develop column are collapsible from the grid header and the status strip. The layout class now supplies only the default: a panel closed to see more of a photograph stays closed while the window keeps its shape, and the choice is dropped when the class changes, because rotating a tablet asks a different question from the one answered in landscape. IMAGE became a Section, being the only group in the column that could not be put away and the one whose content is read first and needed least. Pinch to zoom on the develop canvas, anchored on the midpoint between the fingers (FR-UI-4). The wheel is the desktop's answer and there is no wheel on a tablet. The develop status strip was 28px against the 44px headers on the library and settings pages either side of it — the one screen where a way out has to be found was the one drawn smallest. All three now agree. Verified with cargo test -p dr-ui (192 passing, 6 new), clippy at -D warnings, and an arm64-v8a release build packaged to an APK. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+296
-25
@@ -16,16 +16,16 @@
|
||||
|
||||
mod collections_ui;
|
||||
mod derived_sync;
|
||||
mod net_runtime;
|
||||
mod develop;
|
||||
mod labels;
|
||||
mod library;
|
||||
mod library_ui;
|
||||
#[cfg(live_style)]
|
||||
mod live_style;
|
||||
mod net_runtime;
|
||||
mod settings_store;
|
||||
mod settings_ui;
|
||||
mod trash;
|
||||
#[cfg(live_style)]
|
||||
mod live_style;
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::path::{Path, PathBuf};
|
||||
@@ -63,6 +63,13 @@ const EXPANDED_MIN_WIDTH: f32 = 820.0;
|
||||
/// point where a photographer would notice waiting for the sharp frame.
|
||||
const SETTLE_DELAY: std::time::Duration = std::time::Duration::from_millis(120);
|
||||
|
||||
/// Re-render the current session into the canvas; `true` asks for a draft.
|
||||
///
|
||||
/// Shared rather than passed by reference because most of the callbacks in
|
||||
/// `run` need it and they each outlive the call that built them, so every one
|
||||
/// holds its own handle.
|
||||
type Render = Rc<dyn Fn(&AppWindow, bool)>;
|
||||
|
||||
/// Everything loaded for the currently displayed image.
|
||||
struct Loaded {
|
||||
/// A develop session. `None` only where the file could not be opened for
|
||||
@@ -421,8 +428,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
|
||||
// Set once `show` exists; see where the library grid is wired below.
|
||||
#[allow(clippy::type_complexity)]
|
||||
let open_from_library: Rc<RefCell<Option<Rc<dyn Fn(String)>>>> =
|
||||
Rc::new(RefCell::new(None));
|
||||
let open_from_library: Rc<RefCell<Option<Rc<dyn Fn(String)>>>> = Rc::new(RefCell::new(None));
|
||||
|
||||
// Before anything binds to a token: the compiled palette is already in
|
||||
// place, so this only overwrites what style.yaml currently says.
|
||||
@@ -442,10 +448,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// a folder goes straight to their images (FR-NC-1).
|
||||
{
|
||||
let controller = launch_ui::LaunchController::new();
|
||||
let startup = controller
|
||||
.model
|
||||
.borrow()
|
||||
.startup_action(!paths.is_empty());
|
||||
let startup = controller.model.borrow().startup_action(!paths.is_empty());
|
||||
window.set_show_launch(startup == launch::Startup::ShowLaunchScreen);
|
||||
|
||||
let library = library.clone();
|
||||
@@ -457,18 +460,13 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// A click before then is a no-op rather than a panic — the grid cannot
|
||||
// be reached until the window is running, by which point it is set.
|
||||
let open_from_library = open_from_library.clone();
|
||||
library_ui::wire(
|
||||
&window,
|
||||
library.clone(),
|
||||
collections.clone(),
|
||||
move |path| {
|
||||
let Some(f) = open_from_library.borrow().clone() else {
|
||||
log::warn!("open requested before the viewer was ready: {path}");
|
||||
return;
|
||||
};
|
||||
f(path);
|
||||
},
|
||||
);
|
||||
library_ui::wire(&window, library.clone(), collections.clone(), move |path| {
|
||||
let Some(f) = open_from_library.borrow().clone() else {
|
||||
log::warn!("open requested before the viewer was ready: {path}");
|
||||
return;
|
||||
};
|
||||
f(path);
|
||||
});
|
||||
|
||||
// The collections sidebar shares the library's catalog handle rather
|
||||
// than opening its own: one SQLite connection, so an edit here is
|
||||
@@ -619,7 +617,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
//
|
||||
// Called on every slider change, so it must do no more than run the
|
||||
// adjust pass — the demosaic is not repeated.
|
||||
let render_now: Rc<dyn Fn(&AppWindow, bool)> = {
|
||||
let render_now: Render = {
|
||||
let session = session.clone();
|
||||
let viewport = viewport.clone();
|
||||
Rc::new(move |window: &AppWindow, draft: bool| {
|
||||
@@ -1258,17 +1256,52 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// FR-UI-1: layout class from window width. Computed here rather than in
|
||||
// Slint because a property that both derives from and feeds the layout is
|
||||
// a binding loop.
|
||||
let panels = std::rc::Rc::new(PanelChoices::default());
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let panels = panels.clone();
|
||||
window.on_window_resized(move |width| {
|
||||
let Some(window) = weak.upgrade() else { return };
|
||||
apply_layout_class(&window, width);
|
||||
apply_layout_class(&window, width, &panels);
|
||||
});
|
||||
}
|
||||
{
|
||||
let size = window.window().size();
|
||||
let scale = window.window().scale_factor().max(0.01);
|
||||
apply_layout_class(&window, size.width as f32 / scale);
|
||||
apply_layout_class(&window, size.width as f32 / scale, &panels);
|
||||
}
|
||||
|
||||
// FR-UI-2: the two collapsible columns, opened and closed by hand.
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let panels = panels.clone();
|
||||
window.on_toggle_panel(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let open = !w.get_panel_visible();
|
||||
panels.panel.set(Some(open));
|
||||
w.set_panel_visible(open);
|
||||
});
|
||||
}
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let panels = panels.clone();
|
||||
window.on_toggle_collections(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let open = !w.get_collections_visible();
|
||||
panels.collections.set(Some(open));
|
||||
w.set_collections_visible(open);
|
||||
});
|
||||
}
|
||||
|
||||
// Android's back gesture, and Escape on a keyboard.
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
window.on_back_requested(move || {
|
||||
let Some(w) = weak.upgrade() else {
|
||||
return false;
|
||||
};
|
||||
back_one_step(&w)
|
||||
});
|
||||
}
|
||||
|
||||
if !entries.borrow().is_empty() {
|
||||
@@ -1361,16 +1394,254 @@ fn describe_exposure(m: &Metadata) -> String {
|
||||
parts.join(" ")
|
||||
}
|
||||
|
||||
fn apply_layout_class(window: &AppWindow, width: f32) {
|
||||
/// TRACES: FR-UI-1
|
||||
/// Which collapsible columns the user has opened or closed by hand.
|
||||
///
|
||||
/// The layout class supplies each panel's default; this records where the user
|
||||
/// disagreed, so a panel closed to see more of a photograph stays closed while
|
||||
/// the window keeps its shape.
|
||||
///
|
||||
/// `class` is what makes that "while": a choice is remembered *within* a layout
|
||||
/// class and dropped when the class changes. Rotating a tablet into portrait
|
||||
/// asks a different question from the one answered in landscape, and carrying
|
||||
/// the landscape answer across is how a user ends up with 232px of sidebar on a
|
||||
/// screen that has no room for it and no memory of having asked.
|
||||
#[derive(Default)]
|
||||
struct PanelChoices {
|
||||
class: std::cell::Cell<Option<bool>>,
|
||||
panel: std::cell::Cell<Option<bool>>,
|
||||
collections: std::cell::Cell<Option<bool>>,
|
||||
}
|
||||
|
||||
fn apply_layout_class(window: &AppWindow, width: f32, panels: &PanelChoices) {
|
||||
let expanded = width >= EXPANDED_MIN_WIDTH;
|
||||
window.set_expanded(expanded);
|
||||
window.set_layout_class(if expanded { "expanded" } else { "compact" }.into());
|
||||
|
||||
if panels.class.get() != Some(expanded) {
|
||||
panels.class.set(Some(expanded));
|
||||
panels.panel.set(None);
|
||||
panels.collections.set(None);
|
||||
}
|
||||
|
||||
window.set_panel_visible(panels.panel.get().unwrap_or(expanded));
|
||||
window.set_collections_visible(panels.collections.get().unwrap_or(expanded));
|
||||
}
|
||||
|
||||
/// TRACES: FR-UI-5
|
||||
/// One step back, and whether there was one to take.
|
||||
///
|
||||
/// The Escape half is FR-UI-5's "keyboard shortcuts cover navigation". The
|
||||
/// Android back gesture answers to the same handler and has no numbered
|
||||
/// requirement of its own — the register was written before phones and tablets
|
||||
/// had a platform section, and §1.3 still lists no navigation requirement.
|
||||
///
|
||||
/// This is what Android's back gesture and the Escape key both resolve to. The
|
||||
/// order is the order the states were entered in, innermost first: a mode
|
||||
/// within a view is left before the view is, because that is what the user
|
||||
/// most recently did and so what they most likely mean to undo.
|
||||
///
|
||||
/// Returning `false` means this is the top of the stack. The shell passes that
|
||||
/// straight back to the platform as an unhandled key, which on Android closes
|
||||
/// 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 {
|
||||
let state = NavState {
|
||||
settings: w.get_show_settings(),
|
||||
launch: w.get_show_launch(),
|
||||
browsing: w.get_launch_browsing(),
|
||||
library: w.get_show_library(),
|
||||
crop: w.get_crop_mode(),
|
||||
zoomed: w.get_zoomed(),
|
||||
// Files named on the command line have no grid behind them — the same
|
||||
// condition the status strip uses to decide whether to offer the way
|
||||
// back at all.
|
||||
has_grid: w.get_library_total() > 0,
|
||||
scoped: w.get_collection_selected() != 0,
|
||||
};
|
||||
|
||||
let Some(step) = back_step(state) else {
|
||||
return false;
|
||||
};
|
||||
|
||||
match step {
|
||||
BackStep::CloseSettings => w.invoke_settings_close(),
|
||||
BackStep::CancelBrowse => w.invoke_launch_browse_cancel(),
|
||||
BackStep::LeaveCrop => w.invoke_crop_mode_toggled(false),
|
||||
BackStep::ResetZoom => w.invoke_zoom_reset(),
|
||||
BackStep::ToLibrary => w.invoke_back_to_library(),
|
||||
BackStep::ClearScope => w.invoke_collection_select(0),
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Where the interface is, as far as going back is concerned.
|
||||
///
|
||||
/// A flat snapshot rather than the window itself, so the ordering below can be
|
||||
/// stated and tested without a Slint backend: which of two states is left first
|
||||
/// is the whole of this feature, and it is the part that is easy to get subtly
|
||||
/// wrong when it is spelled out in nested `if`s over live properties.
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
struct NavState {
|
||||
settings: bool,
|
||||
launch: bool,
|
||||
browsing: bool,
|
||||
library: bool,
|
||||
crop: bool,
|
||||
zoomed: bool,
|
||||
has_grid: bool,
|
||||
scoped: bool,
|
||||
}
|
||||
|
||||
/// What one step back does, or `None` at the top of the stack.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
enum BackStep {
|
||||
CloseSettings,
|
||||
CancelBrowse,
|
||||
LeaveCrop,
|
||||
ResetZoom,
|
||||
ToLibrary,
|
||||
ClearScope,
|
||||
}
|
||||
|
||||
fn back_step(s: NavState) -> Option<BackStep> {
|
||||
// Settings is drawn over everything, so it is left first whatever is
|
||||
// behind it.
|
||||
if s.settings {
|
||||
return Some(BackStep::CloseSettings);
|
||||
}
|
||||
|
||||
if s.launch {
|
||||
// The folder picker is a step inside the launch screen; the launch
|
||||
// screen itself is where the application starts and has nothing behind.
|
||||
return s.browsing.then_some(BackStep::CancelBrowse);
|
||||
}
|
||||
|
||||
if !s.library {
|
||||
// Develop. Crop is a mode and zoom is a view state; both are left
|
||||
// before the image is.
|
||||
if s.crop {
|
||||
return Some(BackStep::LeaveCrop);
|
||||
}
|
||||
if s.zoomed {
|
||||
return Some(BackStep::ResetZoom);
|
||||
}
|
||||
return s.has_grid.then_some(BackStep::ToLibrary);
|
||||
}
|
||||
|
||||
// The grid. A collection scoping it is a step in: back widens to the whole
|
||||
// library before it considers leaving.
|
||||
//
|
||||
// And it does not leave: the grid is home, so back from here closes the
|
||||
// application as it does in every other Android app. Signing out is a
|
||||
// deliberate act reached from "Change library", not somewhere a stray swipe
|
||||
// should land.
|
||||
s.scoped.then_some(BackStep::ClearScope)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Develop with a grid behind it — the state most of the back tests vary.
|
||||
fn developing() -> NavState {
|
||||
NavState {
|
||||
has_grid: true,
|
||||
..NavState::default()
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_closes_settings_before_anything_underneath_it() {
|
||||
// Settings is reachable from both the grid and develop, and is drawn
|
||||
// over whichever it was opened from. Whatever is behind must wait.
|
||||
let from_grid = NavState {
|
||||
settings: true,
|
||||
library: true,
|
||||
scoped: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(from_grid), Some(BackStep::CloseSettings));
|
||||
|
||||
let from_develop = NavState {
|
||||
settings: true,
|
||||
crop: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(from_develop), Some(BackStep::CloseSettings));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_leaves_a_mode_before_it_leaves_the_image() {
|
||||
// Crop then zoom then the view: innermost first, because that is the
|
||||
// order they were entered in.
|
||||
let cropping = NavState {
|
||||
crop: true,
|
||||
zoomed: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(cropping), Some(BackStep::LeaveCrop));
|
||||
|
||||
let zoomed = NavState {
|
||||
zoomed: true,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(zoomed), Some(BackStep::ResetZoom));
|
||||
|
||||
assert_eq!(back_step(developing()), Some(BackStep::ToLibrary));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_from_an_image_with_no_grid_behind_it_is_the_top_of_the_stack() {
|
||||
// Files named on the command line: there is no library to return to,
|
||||
// and the status strip does not offer one either.
|
||||
let standalone = NavState {
|
||||
has_grid: false,
|
||||
..developing()
|
||||
};
|
||||
assert_eq!(back_step(standalone), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_widens_a_scoped_grid_before_it_would_leave_the_grid() {
|
||||
let scoped = NavState {
|
||||
library: true,
|
||||
scoped: true,
|
||||
has_grid: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(back_step(scoped), Some(BackStep::ClearScope));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_from_the_whole_grid_closes_the_application() {
|
||||
// The grid is home. Nothing here may navigate to the launch screen:
|
||||
// that is where signing out lives, and a stray back swipe must not
|
||||
// land on it.
|
||||
let home = NavState {
|
||||
library: true,
|
||||
has_grid: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(back_step(home), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn back_cancels_the_folder_picker_but_never_leaves_the_launch_screen() {
|
||||
let picking = NavState {
|
||||
launch: true,
|
||||
browsing: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(back_step(picking), Some(BackStep::CancelBrowse));
|
||||
|
||||
let launch = NavState {
|
||||
launch: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(back_step(launch), None);
|
||||
}
|
||||
|
||||
fn meta() -> Metadata {
|
||||
Metadata {
|
||||
make: Some("Canon".into()),
|
||||
|
||||
Reference in New Issue
Block a user