Merge hitbox improvements #319

Ken Sedgwick (6):
      debug: implemented fmt::Debug for Pubkey
      debug: implemented fmt::Debug for NoteId
      hitbox: more naming cleanup
      hitbox: extend the hitbox to the full width of the container
      hitbox: handle hits on quoted notes correctly
      hitbox: cache note size instead of rect

Closes: https://github.com/damus-io/notedeck/pull/319

enostr/src/note.rs

@@ -1,11 +1,18 @@
 use crate::{Error, Pubkey};
 
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt;
 use std::hash::{Hash, Hasher};
 
-#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
+#[derive(Clone, Copy, Eq, PartialEq, Hash)]
 pub struct NoteId([u8; 32]);
 
+impl fmt::Debug for NoteId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.hex())
+    }
+}
+
 static HRP_NOTE: nostr::bech32::Hrp = nostr::bech32::Hrp::parse_unchecked("note");
 
 impl NoteId {

enostr/src/pubkey.rs

@@ -5,7 +5,7 @@ use nostr::bech32::Hrp;
 use std::fmt;
 use tracing::debug;
 
-#[derive(Debug, Eq, PartialEq, Clone, Copy, Hash)]
+#[derive(Eq, PartialEq, Clone, Copy, Hash)]
 pub struct Pubkey([u8; 32]);
 
 static HRP_NPUB: Hrp = Hrp::parse_unchecked("npub");
@@ -80,6 +80,12 @@ impl fmt::Display for Pubkey {
     }
 }
 
+impl fmt::Debug for Pubkey {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.hex())
+    }
+}
+
 impl From<Pubkey> for String {
     fn from(pk: Pubkey) -> Self {
         pk.hex()

src/ui/note/contents.rs

@@ -1,7 +1,8 @@
+use crate::actionbar::BarAction;
 use crate::images::ImageType;
 use crate::imgcache::ImageCache;
 use crate::notecache::NoteCache;
-use crate::ui::note::NoteOptions;
+use crate::ui::note::{NoteOptions, NoteResponse};
 use crate::ui::ProfilePic;
 use crate::{colors, ui};
 use egui::{Color32, Hyperlink, Image, RichText};
@@ -16,6 +17,7 @@ pub struct NoteContents<'a> {
     note: &'a Note<'a>,
     note_key: NoteKey,
     options: NoteOptions,
+    action: Option<BarAction>,
 }
 
 impl<'a> NoteContents<'a> {
@@ -36,13 +38,18 @@ impl<'a> NoteContents<'a> {
             note,
             note_key,
             options,
+            action: None,
         }
     }
+
+    pub fn action(&self) -> Option<BarAction> {
+        self.action
+    }
 }
 
-impl egui::Widget for NoteContents<'_> {
+impl egui::Widget for &mut NoteContents<'_> {
     fn ui(self, ui: &mut egui::Ui) -> egui::Response {
-        render_note_contents(
+        let result = render_note_contents(
             ui,
             self.ndb,
             self.img_cache,
@@ -51,8 +58,9 @@ impl egui::Widget for NoteContents<'_> {
             self.note,
             self.note_key,
             self.options,
-        )
+        );
-        .response
+        self.action = result.action;
+        result.response
     }
 }
 
@@ -66,7 +74,7 @@ pub fn render_note_preview(
     txn: &Transaction,
     id: &[u8; 32],
     _id_str: &str,
-) -> egui::Response {
+) -> NoteResponse {
     #[cfg(feature = "profiling")]
     puffin::profile_function!();
 
@@ -75,13 +83,13 @@ pub fn render_note_preview(
         if note.kind() == 1 {
             note
         } else {
-            return ui.colored_label(
+            return NoteResponse::new(ui.colored_label(
                 Color32::RED,
                 format!("TODO: can't preview kind {}", note.kind()),
-            );
+            ));
         }
     } else {
-        return ui.colored_label(Color32::RED, "TODO: COULD NOT LOAD");
+        return NoteResponse::new(ui.colored_label(Color32::RED, "TODO: COULD NOT LOAD"));
         /*
         return ui
             .horizontal(|ui| {
@@ -103,19 +111,15 @@ pub fn render_note_preview(
             ui.visuals().noninteractive().bg_stroke.color,
         ))
         .show(ui, |ui| {
-            let resp = ui::NoteView::new(ndb, note_cache, img_cache, &note)
+            ui::NoteView::new(ndb, note_cache, img_cache, &note)
                 .actionbar(false)
                 .small_pfp(true)
                 .wide(true)
                 .note_previews(false)
                 .options_button(true)
-                .show(ui);
+                .show(ui)
-
-            if let Some(context) = resp.context_selection {
-                context.process(ui, &note);
-            }
         })
-        .response
+        .inner
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -128,7 +132,7 @@ fn render_note_contents(
     note: &Note,
     note_key: NoteKey,
     options: NoteOptions,
-) -> egui::InnerResponse<()> {
+) -> NoteResponse {
     #[cfg(feature = "profiling")]
     puffin::profile_function!();
 
@@ -136,7 +140,7 @@ fn render_note_contents(
     let mut images: Vec<String> = vec![];
     let mut inline_note: Option<(&[u8; 32], &str)> = None;
 
-    let resp = ui.horizontal_wrapped(|ui| {
+    let response = ui.horizontal_wrapped(|ui| {
         let blocks = if let Ok(blocks) = ndb.get_blocks_by_key(txn, note_key) {
             blocks
         } else {
@@ -204,9 +208,11 @@ fn render_note_contents(
         }
     });
 
-    if let Some((id, block_str)) = inline_note {
+    let note_action = if let Some((id, block_str)) = inline_note {
-        render_note_preview(ui, ndb, note_cache, img_cache, txn, id, block_str);
+        render_note_preview(ui, ndb, note_cache, img_cache, txn, id, block_str).action
-    }
+    } else {
+        None
+    };
 
     if !images.is_empty() && !options.has_textmode() {
         ui.add_space(2.0);
@@ -215,7 +221,7 @@ fn render_note_contents(
         ui.add_space(2.0);
     }
 
-    resp
+    NoteResponse::new(response.response).with_action(note_action)
 }
 
 fn image_carousel(

src/ui/note/mod.rs

@@ -20,6 +20,7 @@ use crate::{
     notecache::{CachedNote, NoteCache},
     ui::{self, View},
 };
+use egui::emath::{pos2, Vec2};
 use egui::{Id, Label, Pos2, Rect, Response, RichText, Sense};
 use enostr::NoteId;
 use nostrdb::{Ndb, Note, NoteKey, NoteReply, Transaction};
@@ -281,7 +282,7 @@ impl<'a> NoteView<'a> {
                 )
             });
 
-            ui.add(NoteContents::new(
+            ui.add(&mut NoteContents::new(
                 self.ndb,
                 self.img_cache,
                 self.note_cache,
@@ -476,7 +477,7 @@ impl<'a> NoteView<'a> {
                 });
             });
 
-            let resp = ui.add(NoteContents::new(
+            let mut contents = NoteContents::new(
                 self.ndb,
                 self.img_cache,
                 self.note_cache,
@@ -484,10 +485,13 @@ impl<'a> NoteView<'a> {
                 self.note,
                 note_key,
                 self.options(),
-            ));
+            );
+            let resp = ui.add(&mut contents);
+            note_action = note_action.or(contents.action());
 
             if self.options().has_actionbar() {
-                note_action = render_note_actionbar(ui, self.note.id(), note_key).inner;
+                let ab = render_note_actionbar(ui, self.note.id(), note_key);
+                note_action = note_action.or(ab.inner);
             }
 
             resp
@@ -520,7 +524,7 @@ impl<'a> NoteView<'a> {
                         }
                     });
 
-                    ui.add(NoteContents::new(
+                    let mut contents = NoteContents::new(
                         self.ndb,
                         self.img_cache,
                         self.note_cache,
@@ -528,10 +532,13 @@ impl<'a> NoteView<'a> {
                         self.note,
                         note_key,
                         self.options(),
-                    ));
+                    );
+                    ui.add(&mut contents);
+                    note_action = note_action.or(contents.action());
 
                     if self.options().has_actionbar() {
-                        note_action = render_note_actionbar(ui, self.note.id(), note_key).inner;
+                        let ab = render_note_actionbar(ui, self.note.id(), note_key);
+                        note_action = note_action.or(ab.inner);
                     }
                 });
             })
@@ -578,14 +585,23 @@ fn get_reposted_note<'a>(ndb: &Ndb, txn: &'a Transaction, note: &Note) -> Option
 }
 
 fn note_hitbox_id(note_key: NoteKey) -> egui::Id {
-    Id::new(("note_rect", note_key))
+    Id::new(("note_size", note_key))
 }
 
 fn maybe_note_hitbox(ui: &mut egui::Ui, note_key: NoteKey) -> Option<Response> {
     ui.ctx()
         .data_mut(|d| d.get_persisted(note_hitbox_id(note_key)))
-        .map(|rect| {
+        .map(|note_size: Vec2| {
-            let id = ui.make_persistent_id(("under_button_interact", note_key));
+            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();
+            let rect = Rect {
+                min: pos2(container_rect.min.x, container_rect.min.y),
+                max: pos2(container_rect.max.x, container_rect.min.y + note_size.y),
+            };
+
             ui.interact(rect, id, egui::Sense::click())
         })
 }
@@ -599,16 +615,14 @@ fn check_note_hitbox(
     prior_action: Option<BarAction>,
 ) -> Option<BarAction> {
     // Stash the dimensions of the note content so we can render the
-    // underbutton in the next frame
+    // hitbox in the next frame
     ui.ctx().data_mut(|d| {
-        d.insert_persisted(note_hitbox_id(note_key), note_response.rect);
+        d.insert_persisted(note_hitbox_id(note_key), note_response.rect.size());
     });
 
-    // If there was an underbutton and it was clicked open the thread
+    // If there was an hitbox and it was clicked open the thread
     match maybe_hitbox {
-        Some(underbutt) if underbutt.clicked() => {
+        Some(hitbox) if hitbox.clicked() => Some(BarAction::OpenThread(NoteId::new(*note_id))),
-            Some(BarAction::OpenThread(NoteId::new(*note_id)))
-        }
         _ => prior_action,
     }
 }