Maps · View: pins in view take presses, the popup hands focus back

A pin whose place lies outside the view takes no press or Tab, as its
popup would not show. The popup takes focus when it shows and, the
frame it stops showing by its button, Escape, the owner or a pan, hands
focus back to what held it, so focus stays in the map. Its body scrolls
past the room on its side and it paints over the zoom buttons. Tiles
load through gpui's logging loader, so a tile that fails says so. A
drag's owner check has a test, and the per-move viewport logs at debug.
The maps tests split into the map's and the popup's.

src/maps/marker.rs

@@ -1,8 +1,8 @@
 use std::rc::Rc;
 
 use gpui::{
-    AnyElement, App, ElementId, InteractiveElement, IntoElement, ParentElement, Pixels,
-    SharedString, StatefulInteractiveElement, Styled, Window, div, prelude::*,
+    AnyElement, App, ElementId, FocusHandle, InteractiveElement, IntoElement, ParentElement,
+    Pixels, SharedString, StatefulInteractiveElement, Styled, Window, div, prelude::*,
 };
 
 use super::LatLon;
@@ -111,7 +111,7 @@ impl MapMarker {
     }
 }
 
-/// A card that points at a place, over the map; its close button hands the owner `on_close`.
+/// A card that points at a place, over the map. It takes focus when it shows and hands it back when it goes; its close button and Escape hand the owner `on_close`.
 pub struct MapPopup {
     pub(crate) at: LatLon,
     title: SharedString,
@@ -143,12 +143,13 @@ impl MapPopup {
         self.on_close.clone()
     }
 
-    /// The card at its point in a view of `size`; none when the point lies outside.
+    /// The card at its point in a view of `size`, holding `focus`, its body scrolling past the room on its side; none when the point lies outside.
     pub(crate) fn place(
         self,
         owner: &ElementId,
         point: (f32, f32),
         size: (f32, f32),
+        focus: &FocusHandle,
         window: &Window,
         cx: &App,
     ) -> Option<AnyElement> {
@@ -165,44 +166,53 @@ impl MapPopup {
             margin,
             lift,
         )?;
-        let card = div()
-            .id((owner.clone(), "popup"))
-            .absolute()
-            .left(Pixels::from(spot.left))
-            .w(Pixels::from(spot.width))
-            .occlude()
+        let head = div()
             .flex()
-            .items_start()
+            .items_center()
             .gap_2()
-            .p_3()
-            .rounded(theme.radius(Radius::Lg))
-            .bg(colors.overlay)
-            .border_1()
-            .border_color(colors.border)
-            .shadow(theme.elevation(Elevation::Floating))
             .child(
                 div()
                     .flex_1()
                     .min_w_0()
-                    .flex()
-                    .flex_col()
-                    .gap_1()
-                    .child(
-                        div()
-                            .text_size(theme.text_size(TextSize::Sm))
-                            .text_color(colors.fg)
-                            .child(Ellipsis::new(self.title)),
-                    )
-                    .children(self.content),
+                    .text_size(theme.text_size(TextSize::Sm))
+                    .text_color(colors.fg)
+                    .child(Ellipsis::new(self.title)),
             )
-            .when_some(self.on_close, |card, on_close| {
-                card.child(
+            .when_some(self.on_close, |head, on_close| {
+                head.child(
                     IconButton::new((owner.clone(), "popup-close"), IconName::X)
                         .variant(ButtonVariant::Ghost)
                         .tooltip("Close")
                         .on_click(move |_, window, cx| on_close(window, cx)),
                 )
             });
+        let card = div()
+            .id((owner.clone(), "popup"))
+            .track_focus(focus)
+            .absolute()
+            .left(Pixels::from(spot.left))
+            .w(Pixels::from(spot.width))
+            .max_h(Pixels::from(spot.height))
+            .occlude()
+            .flex()
+            .flex_col()
+            .gap_1()
+            .p_3()
+            .rounded(theme.radius(Radius::Lg))
+            .bg(colors.overlay)
+            .border_1()
+            .border_color(colors.border)
+            .focus_ring(cx)
+            .shadow(theme.elevation(Elevation::Floating))
+            .child(head)
+            .children(self.content.map(|content| {
+                div()
+                    .id((owner.clone(), "popup-body"))
+                    .flex_1()
+                    .min_h_0()
+                    .overflow_y_scroll()
+                    .child(content)
+            }));
         Some(
             match spot.edge {
                 Edge::Bottom(bottom) => card.bottom(Pixels::from(bottom)),
@@ -220,11 +230,12 @@ pub(crate) enum Edge {
     Top(f32),
 }
 
-/// Where a popup lies in a view.
+/// Where a popup lies in a view, and the most height it may take there.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub(crate) struct Spot {
     pub left: f32,
     pub width: f32,
+    pub height: f32,
     pub edge: Edge,
 }
 
@@ -241,12 +252,17 @@ pub(crate) fn spot(
     }
     let width = width.min(w - margin * 2.0).max(0.0);
     let left = (x - width / 2.0).clamp(margin, (w - margin - width).max(margin));
-    let edge = if y > h / 2.0 {
-        Edge::Bottom(h - y + lift)
+    let (edge, height) = if y > h / 2.0 {
+        (Edge::Bottom(h - y + lift), y - lift - margin)
     } else {
-        Edge::Top(y + margin)
+        (Edge::Top(y + margin), h - y - margin * 2.0)
     };
-    Some(Spot { left, width, edge })
+    Some(Spot {
+        left,
+        width,
+        height: height.max(0.0),
+        edge,
+    })
 }
 
 #[cfg(test)]
@@ -263,6 +279,7 @@ mod tests {
             Some(Spot {
                 left: 80.0,
                 width: 240.0,
+                height: 210.0,
                 edge: Edge::Bottom(82.0)
             })
         );
@@ -277,9 +294,14 @@ mod tests {
             "held off the right edge"
         );
         assert_eq!(
-            at(200.0, 40.0).map(|spot| spot.edge),
-            Some(Edge::Top(48.0)),
-            "a point up top opens below"
+            at(200.0, 40.0).map(|spot| (spot.edge, spot.height)),
+            Some((Edge::Top(48.0), 244.0)),
+            "a point up top opens below, as tall as the room under it"
+        );
+        assert_eq!(
+            at(200.0, 160.0).map(|spot| spot.height),
+            Some(120.0),
+            "above, the room over the pin"
         );
         assert_eq!(at(-1.0, 100.0), None, "a point outside draws none");
         assert_eq!(at(200.0, 301.0), None);