ui: fix quote repost hitbox

jb55 · jb55 · commit 9bcc338443d4 · 2024-12-03T17:48:36.000-08:00
The response from the wide rendered note was incorrect, leading to and incorrectly sized hitbox. This fixes that. Additionally, we include note options and note parent into the hitbox key, as this may influence the size of the note. Before: https://cdn.jb55.com/s/b2464c22a65adb12.png After: https://cdn.jb55.com/s/52545564d98d278e.png Fixes: #519 Closes: #537 Changelog-Fixed: Fix broken quote repost hitbox Signed-off-by: William Casarin <jb55@jb55.com>
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -26,7 +26,7 @@ eframe = { workspace = true }
 egui_extras = { workspace = true }
 ehttp = "0.2.0"
 egui_tabs = { git = "https://github.com/damus-io/egui-tabs", branch = "egui-0.28" }
-egui_nav = { git = "https://github.com/damus-io/egui-nav", rev = "956338a90e09c7cda951d554626483e0cdbc7825" }
+egui_nav = { git = "https://github.com/damus-io/egui-nav", rev = "fd0900bdff4be35709372e921f2b49f68b261469" }
 egui_virtual_list = { git = "https://github.com/jb55/hello_egui", branch = "egui-0.28", package = "egui_virtual_list" }
 reqwest = { version = "0.12.4", default-features = false, features = [ "rustls-tls-native-roots" ] }
 image = { version = "0.25", features = ["jpeg", "png", "webp"] }
diff --git a/src/ui/note/contents.rs b/src/ui/note/contents.rs
@@ -73,7 +73,7 @@ pub fn render_note_preview(
     img_cache: &mut ImageCache,
     txn: &Transaction,
     id: &[u8; 32],
-    _id_str: &str,
+    parent: NoteKey,
 ) -> NoteResponse {
     #[cfg(feature = "profiling")]
     puffin::profile_function!();
@@ -117,6 +117,7 @@ pub fn render_note_preview(
                 .wide(true)
                 .note_previews(false)
                 .options_button(true)
+                .parent(parent)
                 .show(ui)
         })
         .inner
@@ -213,8 +214,8 @@ fn render_note_contents(
         }
     });
 
-    let note_action = if let Some((id, block_str)) = inline_note {
-        render_note_preview(ui, ndb, note_cache, img_cache, txn, id, block_str).action
+    let note_action = if let Some((id, _block_str)) = inline_note {
+        render_note_preview(ui, ndb, note_cache, img_cache, txn, id, note_key).action
     } else {
         None
     };
diff --git a/src/ui/note/mod.rs b/src/ui/note/mod.rs
@@ -31,6 +31,7 @@ pub struct NoteView<'a> {
     ndb: &'a Ndb,
     note_cache: &'a mut NoteCache,
     img_cache: &'a mut ImageCache,
+    parent: Option<NoteKey>,
     note: &'a nostrdb::Note<'a>,
     flags: NoteOptions,
 }
@@ -195,10 +196,12 @@ impl<'a> NoteView<'a> {
         note: &'a nostrdb::Note<'a>,
     ) -> Self {
         let flags = NoteOptions::actionbar | NoteOptions::note_previews;
+        let parent: Option<NoteKey> = None;
         Self {
             ndb,
             note_cache,
             img_cache,
+            parent,
             note,
             flags,
         }
@@ -257,6 +260,11 @@ impl<'a> NoteView<'a> {
         &mut self.flags
     }
 
+    pub fn parent(mut self, parent: NoteKey) -> Self {
+        self.parent = Some(parent);
+        self
+    }
+
     fn textmode_ui(&mut self, ui: &mut egui::Ui) -> egui::Response {
         let note_key = self.note.key().expect("todo: implement non-db notes");
         let txn = self.note.txn().expect("todo: implement non-db notes");
@@ -440,8 +448,9 @@ impl<'a> NoteView<'a> {
         let mut note_action: Option<NoteAction> = None;
         let mut selected_option: Option<NoteContextSelection> = None;
 
+        let hitbox_id = note_hitbox_id(note_key, self.options(), self.parent);
         let profile = self.ndb.get_profile_by_pubkey(txn, self.note.pubkey());
-        let maybe_hitbox = maybe_note_hitbox(ui, note_key);
+        let maybe_hitbox = maybe_note_hitbox(ui, hitbox_id);
         let container_right = {
             let r = ui.available_rect_before_wrap();
             let x = r.max.x;
@@ -451,64 +460,68 @@ impl<'a> NoteView<'a> {
 
         // wide design
         let response = if self.options().has_wide() {
-            ui.horizontal(|ui| {
-                if self.pfp(note_key, &profile, ui).clicked() {
-                    note_action = Some(NoteAction::OpenProfile(Pubkey::new(*self.note.pubkey())));
-                };
+            ui.vertical(|ui| {
+                ui.horizontal(|ui| {
+                    if self.pfp(note_key, &profile, ui).clicked() {
+                        note_action =
+                            Some(NoteAction::OpenProfile(Pubkey::new(*self.note.pubkey())));
+                    };
 
-                let size = ui.available_size();
-                ui.vertical(|ui| {
-                    ui.add_sized([size.x, self.options().pfp_size()], |ui: &mut egui::Ui| {
-                        ui.horizontal_centered(|ui| {
-                            selected_option = NoteView::note_header(
-                                ui,
-                                self.note_cache,
-                                self.note,
-                                &profile,
-                                self.options(),
-                                container_right,
-                            )
-                            .context_selection;
-                        })
-                        .response
-                    });
+                    let size = ui.available_size();
+                    ui.vertical(|ui| {
+                        ui.add_sized([size.x, self.options().pfp_size()], |ui: &mut egui::Ui| {
+                            ui.horizontal_centered(|ui| {
+                                selected_option = NoteView::note_header(
+                                    ui,
+                                    self.note_cache,
+                                    self.note,
+                                    &profile,
+                                    self.options(),
+                                    container_right,
+                                )
+                                .context_selection;
+                            })
+                            .response
+                        });
 
-                    let note_reply = self
-                        .note_cache
-                        .cached_note_or_insert_mut(note_key, self.note)
-                        .reply
-                        .borrow(self.note.tags());
+                        let note_reply = self
+                            .note_cache
+                            .cached_note_or_insert_mut(note_key, self.note)
+                            .reply
+                            .borrow(self.note.tags());
 
-                    if note_reply.reply().is_some() {
-                        ui.horizontal(|ui| {
-                            reply_desc(ui, txn, &note_reply, self.ndb, self.img_cache);
-                        });
-                    }
+                        if note_reply.reply().is_some() {
+                            ui.horizontal(|ui| {
+                                reply_desc(ui, txn, &note_reply, self.ndb, self.img_cache);
+                            });
+                        }
+                    });
                 });
-            });
 
-            let mut contents = NoteContents::new(
-                self.ndb,
-                self.img_cache,
-                self.note_cache,
-                txn,
-                self.note,
-                note_key,
-                self.options(),
-            );
-            let resp = ui.add(&mut contents);
+                let mut contents = NoteContents::new(
+                    self.ndb,
+                    self.img_cache,
+                    self.note_cache,
+                    txn,
+                    self.note,
+                    note_key,
+                    self.options(),
+                );
 
-            if let Some(action) = contents.action() {
-                note_action = Some(*action);
-            }
+                ui.add(&mut contents);
 
-            if self.options().has_actionbar() {
-                if let Some(action) = render_note_actionbar(ui, self.note.id(), note_key).inner {
-                    note_action = Some(action);
+                if let Some(action) = contents.action() {
+                    note_action = Some(*action);
                 }
-            }
 
-            resp
+                if self.options().has_actionbar() {
+                    if let Some(action) = render_note_actionbar(ui, self.note.id(), note_key).inner
+                    {
+                        note_action = Some(action);
+                    }
+                }
+            })
+            .response
         } else {
             // main design
             ui.with_layout(egui::Layout::left_to_right(egui::Align::TOP), |ui| {
@@ -567,14 +580,11 @@ impl<'a> NoteView<'a> {
             .response
         };
 
-        note_action = check_note_hitbox(
-            ui,
-            self.note.id(),
-            note_key,
-            &response,
-            maybe_hitbox,
-            note_action,
-        );
+        let note_action = if note_hitbox_clicked(ui, hitbox_id, &response.rect, maybe_hitbox) {
+            Some(NoteAction::OpenThread(NoteId::new(*self.note.id())))
+        } else {
+            note_action
+        };
 
         NoteResponse::new(response)
             .with_action(note_action)
@@ -606,16 +616,18 @@ fn get_reposted_note<'a>(ndb: &Ndb, txn: &'a Transaction, note: &Note) -> Option
     note.filter(|note| note.kind() == 1)
 }
 
-fn note_hitbox_id(note_key: NoteKey) -> egui::Id {
-    Id::new(("note_size", note_key))
+fn note_hitbox_id(
+    note_key: NoteKey,
+    note_options: NoteOptions,
+    parent: Option<NoteKey>,
+) -> egui::Id {
+    Id::new(("note_size", note_key, note_options, parent))
 }
 
-fn maybe_note_hitbox(ui: &mut egui::Ui, note_key: NoteKey) -> Option<Response> {
+fn maybe_note_hitbox(ui: &mut egui::Ui, hitbox_id: egui::Id) -> Option<Response> {
     ui.ctx()
-        .data_mut(|d| d.get_persisted(note_hitbox_id(note_key)))
+        .data_mut(|d| d.get_persisted(hitbox_id))
         .map(|note_size: Vec2| {
-            let id = ui.make_persistent_id(("hitbox_interact", note_key));
-
             // The hitbox should extend the entire width of the
             // container.  The hitbox height was cached last layout.
             let container_rect = ui.max_rect();
@@ -624,28 +636,31 @@ fn maybe_note_hitbox(ui: &mut egui::Ui, note_key: NoteKey) -> Option<Response> {
                 max: pos2(container_rect.max.x, container_rect.min.y + note_size.y),
             };
 
-            ui.interact(rect, id, egui::Sense::click())
+            let response = ui.interact(rect, hitbox_id, egui::Sense::click());
+
+            response
+                .widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Other, true, "hitbox"));
+
+            response
         })
 }
 
-fn check_note_hitbox(
+fn note_hitbox_clicked(
     ui: &mut egui::Ui,
-    note_id: &[u8; 32],
-    note_key: NoteKey,
-    note_response: &Response,
+    hitbox_id: egui::Id,
+    note_rect: &Rect,
     maybe_hitbox: Option<Response>,
-    prior_action: Option<NoteAction>,
-) -> Option<NoteAction> {
+) -> bool {
     // Stash the dimensions of the note content so we can render the
     // hitbox in the next frame
     ui.ctx().data_mut(|d| {
-        d.insert_persisted(note_hitbox_id(note_key), note_response.rect.size());
+        d.insert_persisted(hitbox_id, note_rect.size());
     });
 
     // If there was an hitbox and it was clicked open the thread
     match maybe_hitbox {
-        Some(hitbox) if hitbox.clicked() => Some(NoteAction::OpenThread(NoteId::new(*note_id))),
-        _ => prior_action,
+        Some(hitbox) => hitbox.clicked(),
+        _ => false,
     }
 }
 
diff --git a/src/ui/note/post.rs b/src/ui/note/post.rs
@@ -200,7 +200,7 @@ impl<'a> PostView<'a> {
                                                 self.img_cache,
                                                 txn,
                                                 id.bytes(),
-                                                "",
+                                                nostrdb::NoteKey::new(0),
                                             );
                                         });
                                     });
