diff --git a/Nuvolari/README.md b/Nuvolari/README.md index 6206202..3fa1135 100644 --- a/Nuvolari/README.md +++ b/Nuvolari/README.md @@ -103,6 +103,11 @@ Stages whose target does not exist yet are skipped, so this runs from day one. Radar-DPC (CC BY-SA) · Arpa Piemonte · © OpenStreetMap contributors (ODbL) · © OpenMapTiles · OpenFreeMap. +The credits are on the **Sources screen**, reached from the settings button over the +map. There is no permanent credit line on the map itself: the OSMF attribution +guidelines allow the licence information to sit behind an "About option in a menu" so +long as it stays findable, and a test asserts that route still exists. + See [docs/licenses.md](docs/licenses.md) for the obligations these carry — in particular, the rendered radar frames are a derived product of CC BY-SA data and inherit share-alike, and the OpenFreeMap style ships no `attribution` field, so the app has to diff --git a/Nuvolari/app/lib/features/map/attribution_bar.dart b/Nuvolari/app/lib/features/map/attribution_bar.dart deleted file mode 100644 index dfd3e09..0000000 --- a/Nuvolari/app/lib/features/map/attribution_bar.dart +++ /dev/null @@ -1,92 +0,0 @@ -import 'package:flutter/material.dart'; - -import '../../core/region/region_config.dart'; -import '../../l10n/app_localizations.dart'; -import '../sources/sources_screen.dart'; - -/// Always-visible credits for whatever is currently drawn on the map. -/// -/// This is a licence obligation, not decoration: OpenStreetMap's ODbL and -/// Radar-DPC's CC BY-SA both require the credit to be shown wherever the data -/// is. It is therefore never collapsed, hidden behind a gesture, or covered by -/// another control, and it lists only the sources actually on screen — crediting -/// OpenStreetMap while showing the offline fallback would be a false claim. -class AttributionBar extends StatelessWidget { - const AttributionBar({ - required this.region, - required this.activeSourceIds, - this.additionalCredits = const [], - this.showBaseMapNotice = false, - super.key, - }); - - final RegionConfig region; - - /// Attribution ids, matching `attributions[].id` in the region config, for - /// the data currently rendered. - final Set activeSourceIds; - - /// Credits that travel with the data rather than with the region, such as the - /// attribution line inside a radar manifest. Whoever published those frames - /// states there who to credit for them, which is more current than anything - /// baked into the app. - final List additionalCredits; - - /// Whether to say that no base map is configured. True when the offline - /// fallback style is in use, so the placeholder is never mistaken for a map. - final bool showBaseMapNotice; - - @override - Widget build(BuildContext context) { - final l10n = AppLocalizations.of(context); - final theme = Theme.of(context); - - final credits = region.attributions - .where((attribution) => activeSourceIds.contains(attribution.id)) - .map((attribution) => attribution.text) - .toList(growable: false); - - final parts = [ - if (showBaseMapNotice) l10n.baseMapNotConfigured, - ...credits, - ...additionalCredits.where((credit) => credit.isNotEmpty), - ]; - - return Material( - color: theme.colorScheme.surface.withValues(alpha: 0.85), - child: InkWell( - onTap: () => Navigator.of( - context, - ).push(MaterialPageRoute(builder: (_) => const SourcesScreen())), - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 6), - child: Row( - children: [ - Expanded( - child: Text( - parts.join(' · '), - style: theme.textTheme.bodySmall, - maxLines: 2, - overflow: TextOverflow.ellipsis, - ), - ), - const SizedBox(width: 8), - Text( - l10n.sourcesLink, - style: theme.textTheme.bodySmall?.copyWith( - color: theme.colorScheme.primary, - fontWeight: FontWeight.w600, - ), - ), - Icon( - Icons.chevron_right, - size: 16, - color: theme.colorScheme.primary, - ), - ], - ), - ), - ), - ); - } -} diff --git a/Nuvolari/app/lib/features/map/map_overlay_controls.dart b/Nuvolari/app/lib/features/map/map_overlay_controls.dart index db2eeaf..960d1a7 100644 --- a/Nuvolari/app/lib/features/map/map_overlay_controls.dart +++ b/Nuvolari/app/lib/features/map/map_overlay_controls.dart @@ -165,14 +165,20 @@ class TargetSelector extends ConsumerWidget { } } -/// Search and recentre, stacked over the right edge of the map. +/// Settings, search and recentre, stacked over the right edge of the map. class MapActionButtons extends ConsumerWidget { const MapActionButtons({ + required this.onSettings, required this.onSearch, required this.onRecentre, super.key, }); + /// Settings is also where the data credits live, so this button is the route + /// the OSM attribution guidelines require: the licence information stays + /// reachable from the map through a clearly labelled menu. + final VoidCallback onSettings; + final VoidCallback onSearch; final VoidCallback onRecentre; @@ -192,6 +198,13 @@ class MapActionButtons extends ConsumerWidget { return Column( mainAxisSize: MainAxisSize.min, children: [ + FloatingActionButton.small( + heroTag: 'nuvolari-settings', + tooltip: l10n.settingsOpen, + onPressed: onSettings, + child: const Icon(Icons.settings_outlined), + ), + const SizedBox(height: 10), FloatingActionButton.small( heroTag: 'nuvolari-search', tooltip: l10n.searchOpen, diff --git a/Nuvolari/app/lib/features/map/radar_map_screen.dart b/Nuvolari/app/lib/features/map/radar_map_screen.dart index b83dd4f..c17a454 100644 --- a/Nuvolari/app/lib/features/map/radar_map_screen.dart +++ b/Nuvolari/app/lib/features/map/radar_map_screen.dart @@ -22,7 +22,6 @@ import '../settings/settings_screen.dart'; import '../timeline/data_age_banner.dart'; import '../timeline/radar_timeline.dart'; import '../timeline/timeline_bar.dart'; -import 'attribution_bar.dart'; import 'map_overlay_controls.dart'; import 'map_style.dart'; import 'radar_overlay.dart'; @@ -36,23 +35,13 @@ class RadarMapScreen extends ConsumerWidget { final l10n = AppLocalizations.of(context); final region = ref.watch(regionConfigProvider); + // No app bar. Its title was just the app name, which the launcher already + // shows, and its one action now lives over the map with the other controls. + // The map runs full-bleed to the top; the overlaid controls carry their own + // SafeArea. return Scaffold( - appBar: AppBar( - title: Text(l10n.appTitle), - // Only settings here. Everything that changes what the map is looking - // at lives over the map itself, next to the thing it acts on. - actions: [ - IconButton( - tooltip: l10n.settingsOpen, - icon: const Icon(Icons.settings_outlined), - onPressed: () => Navigator.of(context).push( - MaterialPageRoute(builder: (_) => const SettingsScreen()), - ), - ), - ], - ), body: switch (region) { - AsyncData(:final value) => _MapWithAttribution(region: value), + AsyncData(:final value) => _MapBody(region: value), AsyncError(:final error) => Center( child: Padding( padding: const EdgeInsets.all(24), @@ -65,8 +54,8 @@ class RadarMapScreen extends ConsumerWidget { } } -class _MapWithAttribution extends ConsumerWidget { - const _MapWithAttribution({required this.region}); +class _MapBody extends ConsumerWidget { + const _MapBody({required this.region}); final RegionConfig region; @@ -74,40 +63,22 @@ class _MapWithAttribution extends ConsumerWidget { Widget build(BuildContext context, WidgetRef ref) { final l10n = AppLocalizations.of(context); final style = MapStyle.forRegion(region); - final manifest = ref.watch( - radarTimelineProvider.select((state) => state.manifest), - ); return Column( children: [ if (Env.isDemoMode) - MaterialBanner( - content: Text(l10n.demoModeBanner), - actions: const [SizedBox.shrink()], + SafeArea( + bottom: false, + child: MaterialBanner( + content: Text(l10n.demoModeBanner), + actions: const [SizedBox.shrink()], + ), ), Expanded( child: _RegionMap(region: region, style: style), ), const DataAgeBanner(), const TimelineBar(), - // Outside the map rather than floating over it, so the credit can never - // be occluded by a map control. SafeArea keeps it clear of the system - // gesture bar as well — a credit sitting behind the navigation pill is - // a credit that is not being displayed. - SafeArea( - top: false, - child: AttributionBar( - region: region, - activeSourceIds: style.attributionIds, - showBaseMapNotice: style.kind == BaseMapKind.offlineFallback, - // The manifest carries the credit for the frames it indexes, which - // is the whole point of it travelling with the data: whoever - // published these frames says here who to credit for them. - additionalCredits: [ - if (manifest != null) manifest.attribution, - ], - ), - ), ], ); } @@ -381,7 +352,9 @@ class _RegionMapState extends ConsumerState<_RegionMap> { northeast: LatLng(bounds.north, bounds.east), ), ), - attributionButtonPosition: AttributionButtonPosition.bottomRight, + // Top right: the bottom-right corner now holds our own control + // column, and the plugin's button was landing underneath it. + attributionButtonPosition: AttributionButtonPosition.topRight, compassEnabled: false, rotateGesturesEnabled: false, tiltGesturesEnabled: false, @@ -426,6 +399,9 @@ class _RegionMapState extends ConsumerState<_RegionMap> { right: 12, bottom: 12, child: MapActionButtons( + onSettings: () => Navigator.of(context).push( + MaterialPageRoute(builder: (_) => const SettingsScreen()), + ), onSearch: () => unawaited(_openSearch()), onRecentre: () => unawaited(_recentre()), ), diff --git a/Nuvolari/app/lib/features/settings/settings_screen.dart b/Nuvolari/app/lib/features/settings/settings_screen.dart index f10bc2d..c517adf 100644 --- a/Nuvolari/app/lib/features/settings/settings_screen.dart +++ b/Nuvolari/app/lib/features/settings/settings_screen.dart @@ -97,9 +97,14 @@ class _SettingsScreenState extends ConsumerState { ), const Divider(), + // The map no longer carries a permanent credit line, so this is the + // route the attribution guidelines require. It must stay easy to + // find: do not bury it deeper or drop the subtitle that says what it + // holds. ListTile( leading: const Icon(Icons.info_outline), title: Text(l10n.sourcesTitle), + subtitle: Text(l10n.settingsSourcesHint), trailing: const Icon(Icons.chevron_right), onTap: () => Navigator.of(context).push( MaterialPageRoute(builder: (_) => const SourcesScreen()), diff --git a/Nuvolari/app/lib/features/sources/sources_screen.dart b/Nuvolari/app/lib/features/sources/sources_screen.dart index d281446..220a6c2 100644 --- a/Nuvolari/app/lib/features/sources/sources_screen.dart +++ b/Nuvolari/app/lib/features/sources/sources_screen.dart @@ -5,12 +5,18 @@ import 'package:url_launcher/url_launcher.dart'; import '../../core/region/region_config.dart'; import '../../core/region/region_repository.dart'; import '../../l10n/app_localizations.dart'; +import '../timeline/radar_timeline.dart'; /// Sources, licences and the independence disclaimer. /// -/// Reachable from the attribution bar on every screen that shows data. The -/// content is driven by the region config so a new region cannot ship without -/// its credits. +/// Reached from Settings, which is one tap from the map. That path is what the +/// OSM attribution guidelines call for: the credit does not have to sit on the +/// map permanently, but it must stay findable from it — "an (i) button in the +/// corner of the map or an About option in a menu". +/// +/// The content is driven by the region config, so a new region cannot ship +/// without its credits, plus the credit the radar manifest carries for the +/// frames actually on screen. class SourcesScreen extends ConsumerWidget { const SourcesScreen({super.key}); @@ -35,15 +41,21 @@ class SourcesScreen extends ConsumerWidget { } } -class _SourcesBody extends StatelessWidget { +class _SourcesBody extends ConsumerWidget { const _SourcesBody({required this.region}); final RegionConfig region; @override - Widget build(BuildContext context) { + Widget build(BuildContext context, WidgetRef ref) { final l10n = AppLocalizations.of(context); final theme = Theme.of(context); + // The frames carry their own credit, set by whoever published them. It is + // not in the region config, so without this it would be lost when the + // attribution bar came off the map. + final frameCredit = ref.watch( + radarTimelineProvider.select((state) => state.manifest?.attribution), + ); return ListView( padding: const EdgeInsets.symmetric(vertical: 8), @@ -60,6 +72,13 @@ class _SourcesBody extends StatelessWidget { for (final attribution in region.attributions) _AttributionTile(attribution: attribution), + if (frameCredit != null) + ListTile( + leading: const Icon(Icons.radar), + title: Text(frameCredit), + subtitle: Text(l10n.sourcesRadarFrames), + ), + _Card( icon: Icons.copyright_outlined, title: l10n.sourcesShareAlikeTitle, diff --git a/Nuvolari/app/lib/l10n/app_it.arb b/Nuvolari/app/lib/l10n/app_it.arb index 961e778..4f89c48 100644 --- a/Nuvolari/app/lib/l10n/app_it.arb +++ b/Nuvolari/app/lib/l10n/app_it.arb @@ -377,5 +377,13 @@ "sourcesComuniList": "Elenco comuni", "@sourcesComuniList": { "description": "Sources screen heading for the bundled municipality list" + }, + "sourcesRadarFrames": "Credito delle immagini radar attualmente mostrate", + "@sourcesRadarFrames": { + "description": "Subtitle under the credit that the radar manifest carries for its own frames" + }, + "settingsSourcesHint": "Licenze e attribuzioni dei dati mostrati", + "@settingsSourcesHint": { + "description": "Subtitle on the settings entry that opens the Sources screen, so the attribution stays findable from the map" } } diff --git a/Nuvolari/app/test/app_test.dart b/Nuvolari/app/test/app_test.dart index 97ccf9e..7e42295 100644 --- a/Nuvolari/app/test/app_test.dart +++ b/Nuvolari/app/test/app_test.dart @@ -12,7 +12,7 @@ void main() { // The region stays unresolved so the map screen holds its loading state. A // resolved config would instantiate the native map, which has no // implementation in the test harness. - testWidgets('starts up and shows the Italian UI', (tester) async { + testWidgets('starts up in Italian with no app bar', (tester) async { await tester.pumpWidget( ProviderScope( overrides: [ @@ -25,8 +25,10 @@ void main() { ); await tester.pump(); - expect(find.text('Nuvolari'), findsWidgets); expect(find.text('Caricamento…'), findsOneWidget); + // The title bar is gone: the launcher already names the app, and the map + // uses the height instead. + expect(find.byType(AppBar), findsNothing); }); group('AppLocalizations', () { diff --git a/Nuvolari/app/test/features/settings/settings_screen_test.dart b/Nuvolari/app/test/features/settings/settings_screen_test.dart new file mode 100644 index 0000000..5e0080f --- /dev/null +++ b/Nuvolari/app/test/features/settings/settings_screen_test.dart @@ -0,0 +1,115 @@ +import 'dart:io'; + +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:nuvolari/core/region/geo.dart'; +import 'package:nuvolari/core/region/region_config.dart'; +import 'package:nuvolari/core/region/region_repository.dart'; +import 'package:nuvolari/features/places/location_service.dart'; +import 'package:nuvolari/features/places/places_repository.dart'; +import 'package:nuvolari/features/places/saved_places.dart'; +import 'package:nuvolari/features/settings/settings_screen.dart'; +import 'package:nuvolari/features/sources/sources_screen.dart'; +import 'package:nuvolari/l10n/app_localizations.dart'; + +/// Answers without touching the platform channels geolocator needs. +class FakeLocationService implements LocationService { + FakeLocationService(this.state); + + final LocationAvailability state; + + @override + Future availability() async => state; + + @override + Future request() async => state; + + @override + Future currentPoint() async => const GeoPoint(7.686, 45.070); +} + +Widget wrap( + RegionConfig region, { + LocationAvailability availability = LocationAvailability.denied, +}) => ProviderScope( + overrides: [ + regionConfigProvider.overrideWith((ref) async => region), + placesStoreProvider.overrideWithValue(InMemoryPlacesStore()), + locationServiceProvider.overrideWithValue( + FakeLocationService(availability), + ), + ], + child: MaterialApp( + locale: const Locale('it'), + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: const SettingsScreen(), + ), +); + +void main() { + late RegionConfig region; + + setUpAll(() { + region = RegionConfig.parse( + File('assets/regions/piemonte.json').readAsStringSync(), + ); + }); + + group('SettingsScreen', () { + // The map carries no permanent credit line any more, so this entry is the + // route the OSM attribution guidelines require. Losing it would be a + // licence breach, not a cosmetic regression, so it is asserted here. + testWidgets('offers the sources and licences entry', (tester) async { + await tester.pumpWidget(wrap(region)); + await tester.pumpAndSettle(); + + expect(find.text('Fonti e licenze'), findsOneWidget); + expect( + find.text('Licenze e attribuzioni dei dati mostrati'), + findsOneWidget, + ); + }); + + testWidgets('opens the Sources screen from that entry', (tester) async { + await tester.pumpWidget(wrap(region)); + await tester.pumpAndSettle(); + + await tester.tap(find.text('Fonti e licenze')); + await tester.pumpAndSettle(); + + expect(find.byType(SourcesScreen), findsOneWidget); + expect(find.text('App non ufficiale'), findsOneWidget); + }); + + // The disclosure has to be readable after the fact, not only in the moment + // the system dialog is about to appear. + testWidgets('repeats the location disclosure', (tester) async { + await tester.pumpWidget(wrap(region)); + await tester.pumpAndSettle(); + + expect(find.textContaining('resta sul dispositivo'), findsOneWidget); + }); + + testWidgets('reports the permission state', (tester) async { + await tester.pumpWidget( + wrap(region, availability: LocationAvailability.granted), + ); + await tester.pumpAndSettle(); + + expect(find.text('Permesso concesso'), findsOneWidget); + }); + + testWidgets('sends the user to system settings when refused for good', ( + tester, + ) async { + await tester.pumpWidget( + wrap(region, availability: LocationAvailability.deniedForever), + ); + await tester.pumpAndSettle(); + + expect(find.text('Apri impostazioni di sistema'), findsOneWidget); + }); + }); +} diff --git a/Nuvolari/app/test/features/sources/sources_screen_test.dart b/Nuvolari/app/test/features/sources/sources_screen_test.dart index 8e805a1..66881e1 100644 --- a/Nuvolari/app/test/features/sources/sources_screen_test.dart +++ b/Nuvolari/app/test/features/sources/sources_screen_test.dart @@ -5,7 +5,6 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:nuvolari/core/region/region_config.dart'; import 'package:nuvolari/core/region/region_repository.dart'; -import 'package:nuvolari/features/map/attribution_bar.dart'; import 'package:nuvolari/features/sources/sources_screen.dart'; import 'package:nuvolari/l10n/app_localizations.dart'; @@ -51,10 +50,20 @@ void main() { findsOneWidget, reason: '${attribution.id} is missing from the Sources screen', ); - } - expect(find.textContaining('CC BY-SA'), findsWidgets); - expect(find.textContaining('ODbL'), findsWidgets); + // Assert the licence while its row is on screen. Checking at the end + // would only prove whatever the last scroll happened to leave visible. + final license = attribution.license; + expect( + find.text( + license == null + ? 'Nessuna licenza dichiarata dalla fonte' + : 'Licenza: $license', + ), + findsWidgets, + reason: 'the licence for ${attribution.id} is not shown beside it', + ); + } }); // ARPA publishes no licence for the alert bulletin. Showing an invented one @@ -105,63 +114,4 @@ void main() { expect(find.text('Bollettino ufficiale Arpa Piemonte'), findsOneWidget); }); }); - - group('AttributionBar', () { - testWidgets('shows only the sources currently on screen', (tester) async { - await tester.pumpWidget( - wrap( - Scaffold( - body: AttributionBar( - region: region, - activeSourceIds: const {'osm'}, - ), - ), - region, - ), - ); - await tester.pumpAndSettle(); - - expect(find.textContaining('OpenStreetMap'), findsOneWidget); - expect(find.textContaining('Radar-DPC'), findsNothing); - }); - - testWidgets('says when no base map is configured', (tester) async { - await tester.pumpWidget( - wrap( - Scaffold( - body: AttributionBar( - region: region, - activeSourceIds: const {}, - showBaseMapNotice: true, - ), - ), - region, - ), - ); - await tester.pumpAndSettle(); - - expect(find.textContaining('Mappa base non configurata'), findsOneWidget); - }); - - testWidgets('opens the Sources screen when tapped', (tester) async { - await tester.pumpWidget( - wrap( - Scaffold( - body: AttributionBar( - region: region, - activeSourceIds: const {'osm', 'dpc'}, - ), - ), - region, - ), - ); - await tester.pumpAndSettle(); - - await tester.tap(find.byType(AttributionBar)); - await tester.pumpAndSettle(); - - expect(find.byType(SourcesScreen), findsOneWidget); - expect(find.text('App non ufficiale'), findsOneWidget); - }); - }); } diff --git a/Nuvolari/docs/licenses.md b/Nuvolari/docs/licenses.md index 30a19e1..94358a6 100644 --- a/Nuvolari/docs/licenses.md +++ b/Nuvolari/docs/licenses.md @@ -49,17 +49,30 @@ The credit string ARPA asks for is the full *"Fonte: Arpa Piemonte - www.arpa.piemonte.it"*, not the bare name currently in the region config. Adopt the full form the moment any ARPA-sourced data is actually displayed. -## Base map credits are not automatic +## Where the credits live, and why that is enough The OpenFreeMap style JSON has no `attribution` field, so MapLibre displays nothing on -its own. The app renders the credits itself: +its own. The app therefore renders the credits itself, on the **Sources screen**, reached +from the settings button on the map. -- the always-visible attribution bar carries the two **mandatory** credits; -- the Sources screen lists all three with their licences and links. +There is deliberately no permanent credit line over the map. The +[OSMF attribution guidelines](https://osmfoundation.org/wiki/Licence/Attribution_Guidelines) +allow this for a browsable map: attribution may be collapsed or dismissed provided that +*"the user must still be able to find the licence information if they look for it, for +example from an '(i)' button in the corner of the map or an 'About' option in a menu"*. -The bar lists only what is actually on screen. With the offline fallback style there is -no OpenStreetMap data being displayed, so crediting OpenStreetMap there would be a false -attribution — the bar says "Mappa base non configurata" instead. +The route is map → settings → **Fonti e licenze**, two taps and clearly labelled, and +MapLibre's own `(i)` control sits in the map's top-right corner as well. + +**This is a licence obligation, not a layout preference.** Do not bury the Sources entry +deeper, and do not remove its subtitle saying what it contains. A test in +`settings_screen_test.dart` asserts the entry exists and opens the screen, so removing it +fails the build rather than shipping quietly. + +The Sources screen lists every credit the region config declares, **plus** the credit the +radar manifest carries for the frames currently on screen — that one is set by whoever +published the frames and is not in the config, so it would otherwise have been lost when +the on-map bar was removed. ## Advertising, and why it still matters here