Skip to content

Let a placed handover be moved before the rest are placed #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aquasolterra merged 1 commit into from
Sep 18, 2026
Merged

Let a placed handover be moved before the rest are placed #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -997,9 +997,17 @@ UI (features/*/presentation, *widgets)
kind of guess that turns into a wrong answer months later. Insisting also means nothing
downstream needs a rule for dividing a line nobody has said anything about, and no entry
can come out of an import half-placed — the failure that made the rule necessary. A
handover already placed is not final: a further tap moves the nearest one, which is the
screen's only interaction — an earlier "pick it up, then put it down" step was a state
nobody could see, competing with the map for the same tap. The **outer** ends
handover already placed is not final, and the screen has **one rule**: a tap places the
*selected* handover. The selection is a numbered chip in the panel, matched by a larger
numbered mark on the map, and it moves on to the next open handover after each tap — so
the first pass is one tap each — and stays put once none is open, so a second tap refines
the one just placed. It used to be two rules: the open handover took every tap, and only a
finished division let a tap move the *nearest* one, so a handover placed badly could not be
corrected before all the others were placed, and then the nearest could be its neighbour.
The selection lives in the panel and never on the map, because an earlier "pick the mark
up, then put it down" step was a state nobody could see, competing with the map for the
same tap. Snapping still keeps a handover between the nearest *placed* ones on either
side, since with the selection free the one next to it may be open. The **outer** ends
need no asking: the recording's first point is where the first leg started
(`trackImportEnds`), which is also what fills in a single hand-entered leg's coordinates.
An end the user already gave is never overwritten — their statement, with the file as a
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ exact commits.

## Unreleased

- **Importing a recording: a handover can be moved again right away.** Chips under the map
choose which handover the next tap places, so a badly placed one no longer has to wait
until all the others are placed.
- **A new transport entry starts out as a walk** instead of a train.
- **Linux and Windows builds are released too**, next to the Android APKs: an AppImage
and a `.tar.gz` for Linux, and a `.zip` for Windows. The Linux `.tar.gz` carries an
Expand Down
3 changes: 2 additions & 1 deletion docs/features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ trip's ⋮ menu (or from any leg's form), tick the run of entries it covers, and
divided between them. Where an entry has no coordinates, the map asks you to tap the spot where
one leg handed over to the next, and draws the division while you decide; the recording's own
ends fill in the first and last positions, so a single leg usually needs no tapping at all.
Every handover has to be pointed at. Places in the run are given coordinates as well: a place
Every handover has to be pointed at. The chips under the map say which handover a tap
places; tap a chip to go back and move one you already placed. Places in the run are given coordinates as well: a place
standing between two legs is their handover, so it gets the same spot they do, and one at either
end of the run gets where the recording started or finished. Anything you had already placed
yourself is left alone.
Expand Down
184 changes: 129 additions & 55 deletions lib/features/map/presentation/track_import_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,22 @@ const Color kTrackUndividedColor = Color(0xFF9E9E9E);
/// turns into a wrong answer months later. It also means nothing downstream
/// needs a rule for dividing a line nobody has said anything about.
///
/// There is one interaction and no modes: **a tap puts a handover where it
/// landed.** While one is still open it is that one; once they are all placed a
/// tap moves the nearest, which is what changing your mind looks like. An
/// earlier version made the marker itself tappable to "pick up" first — an
/// invisible state, and one that competed with the map for the same tap, so
/// often neither happened. The marks are inert on purpose, and the tap is the
/// map's own `onTap`: flutter_map's gesture handling wraps its children, so a
/// There is one rule: **a tap puts the selected handover where it landed.**
/// Which one is selected is on screen — a numbered chip under the map, and the
/// matching mark on it drawn larger — and a tap on a chip selects another. After
/// a handover is placed the selection moves on to the next one still open, so
/// the first pass is one tap per handover; once none is open it stays where it
/// is, so tapping again refines the one just placed. Re-placing an earlier one
/// before the rest are done is a tap on its chip, where it used to be
/// impossible: open ones always took the tap, and only a finished division let
/// the nearest one move — a second rule, and one that could pick the neighbour
/// of the handover you meant.
///
/// The selection is chosen in the panel and never on the map. An earlier
/// version made the marker itself tappable to "pick up" first — an invisible
/// state, and one that competed with the map for the same tap, so often neither
/// happened. The marks are inert on purpose, and the tap is the map's own
/// `onTap`: flutter_map's gesture handling wraps its children, so a
/// `GestureDetector` placed among them never wins the arena and no tap arrives
/// at all. Both of those were tried; neither is worth trying again.
Future<bool?> importTrackAcrossEntries(
Expand Down Expand Up @@ -94,6 +103,9 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {
late TrackImportPlan _plan;
late List<LatLng?> _boundaries;

/// The handover the next tap places, or null when there is none to place.
int? _selected;

/// Every point of the recording in one sequence — what a tap is snapped
/// against, and the same order the cutting walks.
late List<LatLng> _flat;
Expand All @@ -104,6 +116,7 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {
_plan = trackImportPlan(widget.selection);
_boundaries = [..._plan.boundaries];
_flat = [for (final line in widget.lines) ...line];
_selected = _open ?? (_boundaries.isEmpty ? null : 0);
}

/// The first handover nobody has said anything about, or null once they are
Expand All @@ -129,43 +142,46 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {

bool get _complete => _boundaries.every((b) => b != null);

/// A tap puts a handover where it landed. That is the whole interaction.
/// A tap puts the selected handover where it landed. That is the whole
/// interaction.
///
/// While one is still open it is that one; once they are all placed a tap
/// moves the **nearest**, which is what changing your mind looks like. There
/// is deliberately no picking-up step: a mode you cannot see is a mode you
/// cannot use, and nothing here is written until *Import* anyway — so a tap in
/// the wrong place costs another tap and is visible the moment it happens,
/// There is deliberately no picking-up step on the map: a mode you cannot see
/// is a mode you cannot use, and the selection is drawn in the panel and on
/// the mark. Nothing here is written until *Import* anyway — so a tap in the
/// wrong place costs another tap and is visible the moment it happens,
/// because the division is redrawn under it.
void _place(LatLng tap) {
final index = _open ?? _nearest(tap);
final index = _selected;
if (index == null) return;

// Between its neighbours, because a stretch cannot run backwards.
final previous = index == 0 ? null : _boundaries[index - 1];
final next = index + 1 < _boundaries.length ? _boundaries[index + 1] : null;
// Between the nearest placed handovers on either side, because a stretch
// cannot run backwards. Not merely the immediate neighbours: with the
// selection free, the one next to it may still be open.
LatLng? previous;
for (var i = index - 1; i >= 0 && previous == null; i--) {
previous = _boundaries[i];
}
LatLng? next;
for (var i = index + 1; i < _boundaries.length && next == null; i++) {
next = _boundaries[i];
}
final after = previous == null ? 0 : trackIndexOf(_flat, previous) + 1;
final before = next == null ? null : trackIndexOf(_flat, next) - 1;
final snapped = snapToTrack(_flat, tap, after: after, before: before);
if (snapped == null) return;
setState(() => _boundaries[index] = snapped);
setState(() {
_boundaries[index] = snapped;
_selected = _nextOpen(index) ?? index;
});
}

/// The placed handover closest to [tap].
int? _nearest(LatLng tap) {
const distance = Distance(calculator: Haversine());
var best = -1;
var bestMetres = double.infinity;
for (var i = 0; i < _boundaries.length; i++) {
final at = _boundaries[i];
if (at == null) continue;
final metres = distance.as(LengthUnit.Meter, at, tap);
if (metres < bestMetres) {
bestMetres = metres;
best = i;
}
/// The first handover still open after [index], else the first one before
/// it — or null when none is.
int? _nextOpen(int index) {
for (var i = index + 1; i < _boundaries.length; i++) {
if (_boundaries[i] == null) return i;
}
return best < 0 ? null : best;
return _open;
}

Future<void> _confirm() async {
Expand Down Expand Up @@ -214,7 +230,7 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {
final l10n = AppLocalizations.of(context);
final theme = Theme.of(context);
const basemap = kDefaultBasemap;
final open = _open;
final selected = _selected;
final placed = _placedPrefix;
final stretches = splitTracks(widget.lines, placed);

Expand Down Expand Up @@ -276,14 +292,19 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {
if (_boundaries[i] case final at?)
Marker(
point: at,
width: 28,
height: 28,
width: 32,
height: 32,
// Ignoring pointers on purpose: the mark is a
// mark, not a control. Letting it take taps put it
// in competition with the layer that places
// handovers, and a tap that two widgets both want
// is a tap that does nothing.
child: const IgnorePointer(child: _Handover()),
child: IgnorePointer(
child: _Handover(
number: i + 1,
selected: i == selected,
),
),
),
],
),
Expand Down Expand Up @@ -313,13 +334,17 @@ class _TrackImportScreenState extends ConsumerState<TrackImportScreen> {
_Panel(
l10n: l10n,
theme: theme,
question: open == null
question: selected == null
? null
: l10n.trackTapBoundary(
_label(_plan.legs[open], l10n),
_label(_plan.legs[open + 1], l10n),
: (_boundaries[selected] == null
? l10n.trackTapBoundary
: l10n.trackMoveBoundary)(
_label(_plan.legs[selected], l10n),
_label(_plan.legs[selected + 1], l10n),
),
hint: _complete ? l10n.trackBoundaryMove : null,
handovers: [for (final at in _boundaries) at != null],
selected: selected,
onSelect: (index) => setState(() => _selected = index),
legend: [
for (var i = 0; i < _plan.legs.length; i++)
(
Expand Down Expand Up @@ -357,7 +382,9 @@ class _Panel extends StatelessWidget {
required this.l10n,
required this.theme,
required this.question,
required this.hint,
required this.handovers,
required this.selected,
required this.onSelect,
required this.legend,
required this.summary,
required this.onConfirm,
Expand All @@ -366,7 +393,11 @@ class _Panel extends StatelessWidget {
final AppLocalizations l10n;
final ThemeData theme;
final String? question;
final String? hint;

/// One per handover, saying whether it has been placed.
final List<bool> handovers;
final int? selected;
final ValueChanged<int> onSelect;
final List<(String, Color)> legend;
final String summary;
final VoidCallback? onConfirm;
Expand Down Expand Up @@ -396,15 +427,42 @@ class _Panel extends StatelessWidget {
),
],
),
const SizedBox(height: 8),
Text(
question ?? hint ?? summary,
style: theme.textTheme.bodyMedium?.copyWith(
color: question == null
? theme.colorScheme.onSurfaceVariant
: theme.colorScheme.onSurface,
if (handovers.isNotEmpty) ...[
const SizedBox(height: 8),
// Scrolls rather than wraps: a long run must not eat the map.
SingleChildScrollView(
scrollDirection: Axis.horizontal,
child: Row(
children: [
for (var i = 0; i < handovers.length; i++)
Padding(
padding: const EdgeInsetsDirectional.only(end: 8),
child: ChoiceChip(
avatar: Icon(
handovers[i]
? Icons.check_circle
: Icons.radio_button_unchecked,
size: 18,
),
showCheckmark: false,
label: Text(l10n.trackHandoverChip(i + 1)),
selected: i == selected,
onSelected: (_) => onSelect(i),
),
),
],
),
),
),
],
if (question case final question?) ...[
const SizedBox(height: 8),
Text(
question,
style: theme.textTheme.bodyMedium?.copyWith(
color: theme.colorScheme.onSurface,
),
),
],
const SizedBox(height: 8),
Row(
children: [
Expand Down Expand Up @@ -434,18 +492,34 @@ class _Panel extends StatelessWidget {
/// Ink on a halo, in its own colours rather than the theme's — raster tiles are
/// pale in both themes and full of thin lines, so a mark tinted by the theme
/// reads as a road.
///
/// Numbered, so it can be matched to its chip in the panel; the selected one is
/// drawn larger, which is the map's half of saying which one a tap will move.
class _Handover extends StatelessWidget {
const _Handover();
const _Handover({required this.number, required this.selected});

final int number;
final bool selected;

@override
Widget build(BuildContext context) => Center(
child: Container(
width: 18,
height: 18,
width: selected ? 30 : 22,
height: selected ? 30 : 22,
alignment: Alignment.center,
decoration: BoxDecoration(
color: const Color(0xFF212121),
shape: BoxShape.circle,
border: Border.all(color: Colors.white, width: 3),
border: Border.all(color: Colors.white, width: selected ? 4 : 2),
),
child: Text(
'$number',
style: TextStyle(
color: Colors.white,
fontSize: selected ? 13 : 11,
fontWeight: FontWeight.bold,
height: 1,
),
),
),
);
Expand Down
3 changes: 2 additions & 1 deletion lib/l10n/app_de.arb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,8 @@
"trackPickOption": "Welcher Option ist die Linie gefolgt?",
"trackOptionNotChosen": "Nicht die Option, der die Reise folgt",
"trackTapBoundary": "Antippen, wo \u201e{before}\u201c an \u201e{after}\u201c übergibt",
"trackBoundaryMove": "Ein Tipp verschiebt den nächsten Übergabepunkt",
"trackMoveBoundary": "Antippen, um zu verschieben, wo \u201e{before}\u201c an \u201e{after}\u201c übergibt",
"trackHandoverChip": "Übergabe {number}",
"trackImportConfirm": "Importieren",
"trackImportSummary": "{legs, plural, =1{1 Eintrag} other{{legs} Einträge}}, {ends, plural, =0{keine Koordinaten gesetzt} =1{1 Koordinate gesetzt} other{{ends} Koordinaten gesetzt}}",
"trackNoLegsPicked": "Mindestens einen Transport-Eintrag wählen",
Expand Down
12 changes: 9 additions & 3 deletions lib/l10n/app_en.arb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,15 @@
"description": "Asks the user to point at the handover between two legs.",
"placeholders": { "before": { "type": "String" }, "after": { "type": "String" } }
},
"trackBoundaryMove": "Tapping moves the nearest handover",
"@trackBoundaryMove": {
"description": "Says that a handover already placed is not final."
"trackMoveBoundary": "Tap to move where \u201c{before}\u201d hands over to \u201c{after}\u201d",
"@trackMoveBoundary": {
"description": "The selected handover is already placed; a tap moves it.",
"placeholders": { "before": { "type": "String" }, "after": { "type": "String" } }
},
"trackHandoverChip": "Handover {number}",
"@trackHandoverChip": {
"description": "Chip selecting which handover the next tap on the map places.",
"placeholders": { "number": { "type": "int" } }
},
"trackImportConfirm": "Import",
"@trackImportConfirm": {
Expand Down
12 changes: 9 additions & 3 deletions lib/l10n/app_localizations.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1874,11 +1874,17 @@ abstract class AppLocalizations {
/// **'Tap where “{before}” hands over to “{after}”'**
String trackTapBoundary(String before, String after);

/// Says that a handover already placed is not final.
/// The selected handover is already placed; a tap moves it.
///
/// In en, this message translates to:
/// **'Tapping moves the nearest handover'**
String get trackBoundaryMove;
/// **'Tap to move where “{before}” hands over to “{after}”'**
String trackMoveBoundary(String before, String after);

/// Chip selecting which handover the next tap on the map places.
///
/// In en, this message translates to:
/// **'Handover {number}'**
String trackHandoverChip(int number);

/// Writes the divided line onto the entries.
///
Expand Down
10 changes: 8 additions & 2 deletions lib/l10n/app_localizations_de.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1070,8 +1070,14 @@ class AppLocalizationsDe extends AppLocalizations {
}

@override
String get trackBoundaryMove =>
'Ein Tipp verschiebt den nächsten Übergabepunkt';
String trackMoveBoundary(String before, String after) {
return 'Antippen, um zu verschieben, wo „$before“ an „$after“ übergibt';
}

@override
String trackHandoverChip(int number) {
return 'Übergabe $number';
}

@override
String get trackImportConfirm => 'Importieren';
Expand Down
Loading
Loading