[fractal] room-history: Show location viewer error when geo URI parsing fails



commit 76ddc9c45afa0c87520b8bdfc7efb9fd940a9cbb
Author: Paul van Tilburg <paul luon net>
Date:   Sun Oct 2 15:34:16 2022 +0200

    room-history: Show location viewer error when geo URI parsing fails
    
    This is only used for displaying locations. When previewing/sending a
    location, it is assumed to be valid, so no error overlay is necessary.
    
    * Pass a `GeoUri` object to the `LoctionViewer` and `MediaContentViewer`
      instead of a geo URI string so it is known to be valid
    * Wrap the `LocationViewer` component in a `GtkOverlay` in the
      `ContentMessageLocation` component that can overlay an error message
      over the location viewer if coordinate parsing fails

 data/resources/ui/content-message-location.ui      | 22 ++++++++++++-
 po/POTFILES.in                                     |  1 +
 src/components/location_viewer.rs                  | 11 +++----
 src/components/media_content_viewer.rs             |  5 +--
 .../content/room_history/attachment_dialog.rs      |  6 +++-
 .../content/room_history/message_row/location.rs   | 37 +++++++++++++++++-----
 src/session/content/room_history/mod.rs            |  8 ++---
 7 files changed, 67 insertions(+), 23 deletions(-)
---
diff --git a/data/resources/ui/content-message-location.ui b/data/resources/ui/content-message-location.ui
index 607b13976..22b88afed 100644
--- a/data/resources/ui/content-message-location.ui
+++ b/data/resources/ui/content-message-location.ui
@@ -6,7 +6,27 @@
     </style>
     <property name="overflow">hidden</property>
     <child>
-      <object class="ComponentsLocationViewer" id="location"/>
+      <object class="GtkOverlay" id="overlay">
+        <property name="valign">center</property>
+        <child>
+          <object class="ComponentsLocationViewer" id="location"/>
+        </child>
+        <child type="overlay">
+          <object class="GtkImage" id="overlay_error">
+            <style>
+              <class name="osd"/>
+              <class name="circular"/>
+            </style>
+            <property name="halign">center</property>
+            <property name="valign">center</property>
+            <property name="icon-name">dialog-error-symbolic</property>
+            <property name="icon-size">large</property>
+            <layout>
+              <property name="measure">true</property>
+            </layout>
+          </object>
+        </child>
+      </object>
     </child>
   </template>
 </interface>
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 12ef17006..73ed8c11d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -66,6 +66,7 @@ src/session/content/room_details/mod.rs
 src/session/content/room_history/item_row.rs
 src/session/content/room_history/message_row/audio.rs
 src/session/content/room_history/message_row/content.rs
+src/session/content/room_history/message_row/location.rs
 src/session/content/room_history/message_row/media.rs
 src/session/content/room_history/mod.rs
 src/session/content/room_history/state_row/creation.rs
diff --git a/src/components/location_viewer.rs b/src/components/location_viewer.rs
index b18e11b55..19777e32b 100644
--- a/src/components/location_viewer.rs
+++ b/src/components/location_viewer.rs
@@ -132,14 +132,11 @@ impl LocationViewer {
         self.notify("compact");
     }
 
-    pub fn set_geo_uri(&self, uri: &str) {
+    // Move the map viewport to the provided coordinates and draw a marker.
+    pub fn set_location(&self, geo_uri: &GeoUri) {
         let imp = self.imp();
-
-        let (latitude, longitude) = match GeoUri::parse(uri) {
-            Ok(geo_uri) => (geo_uri.latitude(), geo_uri.longitude()),
-            // FIXME: Actually handle the error by showing it instead of the map.
-            Err(_) => return,
-        };
+        let latitude = geo_uri.latitude();
+        let longitude = geo_uri.longitude();
 
         imp.map
             .viewport()
diff --git a/src/components/media_content_viewer.rs b/src/components/media_content_viewer.rs
index 807a06b22..cf53c3720 100644
--- a/src/components/media_content_viewer.rs
+++ b/src/components/media_content_viewer.rs
@@ -1,4 +1,5 @@
 use adw::{prelude::*, subclass::prelude::*};
+use geo_uri::GeoUri;
 use gettextrs::gettext;
 use gtk::{gdk, gio, glib, glib::clone, CompositeTemplate};
 use log::warn;
@@ -292,7 +293,7 @@ impl MediaContentViewer {
     }
 
     /// View the given location as a geo URI.
-    pub fn view_location(&self, geo_uri: &str) {
+    pub fn view_location(&self, geo_uri: &GeoUri) {
         self.show_loading();
 
         let priv_ = self.imp();
@@ -309,7 +310,7 @@ impl MediaContentViewer {
             location
         };
 
-        location.set_geo_uri(geo_uri);
+        location.set_location(geo_uri);
         self.show_viewer();
     }
 }
diff --git a/src/session/content/room_history/attachment_dialog.rs 
b/src/session/content/room_history/attachment_dialog.rs
index 272f5ea4c..069f4ca7e 100644
--- a/src/session/content/room_history/attachment_dialog.rs
+++ b/src/session/content/room_history/attachment_dialog.rs
@@ -77,7 +77,11 @@ impl AttachmentDialog {
         obj
     }
 
-    pub fn for_location(transient_for: &gtk::Window, title: &str, geo_uri: &str) -> Self {
+    pub fn for_location(
+        transient_for: &gtk::Window,
+        title: &str,
+        geo_uri: &geo_uri::GeoUri,
+    ) -> Self {
         let obj: Self = glib::Object::new(&[("transient-for", transient_for), ("title", &title)])
             .expect("Failed to create AttachmentDialog");
         obj.imp().media.view_location(geo_uri);
diff --git a/src/session/content/room_history/message_row/location.rs 
b/src/session/content/room_history/message_row/location.rs
index ab903ce78..fc875d739 100644
--- a/src/session/content/room_history/message_row/location.rs
+++ b/src/session/content/room_history/message_row/location.rs
@@ -1,5 +1,8 @@
 use adw::{prelude::*, subclass::prelude::*};
+use geo_uri::GeoUri;
+use gettextrs::gettext;
 use gtk::{glib, CompositeTemplate};
+use log::warn;
 
 use super::ContentFormat;
 use crate::components::LocationViewer;
@@ -12,8 +15,12 @@ mod imp {
     #[derive(Debug, Default, CompositeTemplate)]
     #[template(resource = "/org/gnome/Fractal/content-message-location.ui")]
     pub struct MessageLocation {
+        #[template_child]
+        pub overlay: TemplateChild<gtk::Overlay>,
         #[template_child]
         pub location: TemplateChild<LocationViewer>,
+        #[template_child]
+        pub overlay_error: TemplateChild<gtk::Image>,
     }
 
     #[glib::object_subclass]
@@ -33,7 +40,7 @@ mod imp {
 
     impl ObjectImpl for MessageLocation {
         fn dispose(&self, _obj: &Self::Type) {
-            self.location.unparent();
+            self.overlay.unparent();
         }
     }
 
@@ -61,7 +68,7 @@ mod imp {
             } else {
                 width
             };
-            self.location
+            self.overlay
                 .size_allocate(&gtk::Allocation::new(0, 0, width, height), baseline)
         }
     }
@@ -81,11 +88,25 @@ impl MessageLocation {
     }
 
     pub fn set_geo_uri(&self, uri: &str, format: ContentFormat) {
-        let location = &self.imp().location;
-        location.set_geo_uri(uri);
-        location.set_compact(matches!(
-            format,
-            ContentFormat::Compact | ContentFormat::Ellipsized
-        ))
+        let priv_ = self.imp();
+
+        match GeoUri::parse(uri) {
+            Ok(geo_uri) => {
+                priv_.location.set_compact(matches!(
+                    format,
+                    ContentFormat::Compact | ContentFormat::Ellipsized
+                ));
+                priv_.location.set_location(&geo_uri);
+                priv_.overlay_error.hide();
+            }
+            Err(error) => {
+                warn!("Encountered invalid geo URI: {}", error);
+                priv_.location.hide();
+                priv_.overlay_error.set_tooltip_text(Some(&gettext(
+                    "Location is invalid and cannot be displayed",
+                )));
+                priv_.overlay_error.show();
+            }
+        };
     }
 }
diff --git a/src/session/content/room_history/mod.rs b/src/session/content/room_history/mod.rs
index f619961a1..abdc1e7d1 100644
--- a/src/session/content/room_history/mod.rs
+++ b/src/session/content/room_history/mod.rs
@@ -927,8 +927,7 @@ impl RoomHistory {
                 .latitude(location.latitude())
                 .longitude(location.longitude())
                 .build()
-                .expect("Got invalid coordinates from ashpd")
-                .to_string();
+                .expect("Got invalid coordinates from ashpd");
 
             let window = self.root().unwrap().downcast::<gtk::Window>().unwrap();
             let dialog =
@@ -937,13 +936,14 @@ impl RoomHistory {
                 return Ok(());
             }
 
+            let geo_uri_string = geo_uri.to_string();
             let iso8601_datetime =
                 glib::DateTime::from_unix_local(location.timestamp().as_secs() as i64)
                     .expect("Valid location timestamp");
             let location_body = gettext_f(
                 "User Location {geo_uri} at {iso8601_datetime}",
                 &[
-                    ("geo_uri", &geo_uri),
+                    ("geo_uri", &geo_uri_string),
                     (
                         "iso8601_datetime",
                         iso8601_datetime.format_iso8601().unwrap().as_str(),
@@ -952,7 +952,7 @@ impl RoomHistory {
             );
             room.send_room_message_event(AnyMessageLikeEventContent::RoomMessage(
                 RoomMessageEventContent::new(MessageType::Location(
-                    LocationMessageEventContent::new(location_body, geo_uri),
+                    LocationMessageEventContent::new(location_body, geo_uri_string),
                 )),
             ));
         }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]