From f6bbfe61daac6e882d463aac0adb9e08dec8fd32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 05:22:50 -0400 Subject: [PATCH] T22 round 1 fixes - a remote cursor shows only while its caret sits inside the field, sideways too - presence avatars keep a transparent border for the focus ring - the access list shows each role, as words when it cannot change - status dots take the theme's size - peer and member pictures load from files and web addresses alike --- TASKS.md | 1 + src/collab/peers.rs | 18 +++++++++++++----- src/collab/remote.rs | 38 +++++++++++++++++++++++++++++++++++--- src/collab/share.rs | 27 ++++++++++++++++++++++----- src/documents/mod.rs | 1 + 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/TASKS.md b/TASKS.md index bcbfe18..a044d7a 100644 --- a/TASKS.md +++ b/TASKS.md @@ -254,3 +254,4 @@ Per-component lines live in `tasks/`. Tags there: | T21c | 1 | FAIL | the gallery's page properties dropped a new due date and tags | | T21b | 2 | PASS | | | T21c | 2 | PASS | | +| T22 | 1 | FAIL | a remote cursor scrolled out sideways still drew its flag; presence avatars showed no focus ring; the access list hid roles it could not change; status dots used a fixed size | diff --git a/src/collab/peers.rs b/src/collab/peers.rs index 670c2e0..6126435 100644 --- a/src/collab/peers.rs +++ b/src/collab/peers.rs @@ -1,4 +1,4 @@ -use std::{path::Path, rc::Rc, time::Duration}; +use std::{rc::Rc, time::Duration}; use gpui::{ Animation, AnimationExt, AnyElement, App, ElementId, FontWeight, Hsla, InteractiveElement, @@ -10,6 +10,7 @@ use smallvec::SmallVec; use crate::{ buttons::{Button, ButtonVariant}, data_display::{Avatar, more, tucked}, + documents::source, forms::{OnValue, Run}, overlays::HoverCard, primitives::{FocusRing, hand_back, hold_focus}, @@ -53,7 +54,7 @@ impl Peer { pub(crate) fn avatar(&self, id: impl Into, size: AvatarSize) -> Avatar { let avatar = Avatar::new(id, self.name.clone()).size(size); match &self.picture { - Some(picture) => avatar.image(Path::new(picture.as_ref())), + Some(picture) => avatar.image(source(picture)), None => avatar, } } @@ -116,6 +117,8 @@ impl RenderOnce for PresenceAvatars { let face = tucked(ix == 0, size) .id((self.id.clone(), format!("peer-{key}"))) .rounded_full() + .border_1() + .border_color(transparent_black()) .tab_index(0) .focus_ring(cx) .cursor_pointer() @@ -145,7 +148,12 @@ impl RenderOnce for PresenceAvatars { .flex() .items_center() .gap_1p5() - .child(div().size_2().rounded_full().bg(color)) + .child( + div() + .size(cx.theme().status_dot()) + .rounded_full() + .bg(color), + ) .child( div().font_weight(FontWeight::MEDIUM).child(name.clone()), ), @@ -197,7 +205,7 @@ impl RenderOnce for LiveIndicator { let theme = cx.theme(); let colors = theme.colors.clone(); let live = self.others > 0; - let dot = div().size_2().rounded_full().bg(if live { + let dot = div().size(theme.status_dot()).rounded_full().bg(if live { colors.success } else { colors.fg_subtle @@ -303,7 +311,7 @@ impl RenderOnce for FollowMode { .rounded_t(theme.radius(Radius::Md)) .bg(color.opacity(TINT)) .text_size(theme.text_size(TextSize::Sm)) - .child(div().size_2().rounded_full().bg(color)) + .child(div().size(theme.status_dot()).rounded_full().bg(color)) .child( div() .flex_1() diff --git a/src/collab/remote.rs b/src/collab/remote.rs index 4a7807b..0f643c7 100644 --- a/src/collab/remote.rs +++ b/src/collab/remote.rs @@ -1,8 +1,8 @@ use std::ops::Range; use gpui::{ - App, ContentMask, Entity, FontWeight, IntoElement, RenderOnce, SharedString, Styled, TextRun, - Window, canvas, fill, point, size, + App, Bounds, ContentMask, Entity, FontWeight, IntoElement, Pixels, RenderOnce, SharedString, + Styled, TextRun, Window, canvas, fill, point, size, }; use super::peers::Peer; @@ -15,6 +15,14 @@ use crate::{ /// How strongly a peer's color washes their selection. const WASH: f32 = 0.2; +/// Whether a caret shows inside the field's box. +pub(crate) fn in_view(field: Bounds, caret: Bounds) -> bool { + caret.bottom() > field.top() + && caret.top() < field.bottom() + && caret.left() >= field.left() + && caret.left() < field.right() +} + /// Someone else's caret in a shared field: a bar in their color, their name on a flag above. Lay it over the field, in a box its size; it reads where `offset` sits as the field paints. #[derive(IntoElement)] pub struct RemoteCursor { @@ -49,7 +57,7 @@ impl RenderOnce for RemoteCursor { let Some(caret) = input.read(cx).bounds_for(offset) else { return; }; - if caret.bottom() <= bounds.top() || caret.top() >= bounds.bottom() { + if !in_view(bounds, caret) { return; } window.with_content_mask(Some(ContentMask { bounds }), |window| { @@ -133,3 +141,27 @@ impl RenderOnce for RemoteSelection { .inset_0() } } + +#[cfg(test)] +mod tests { + use gpui::{Bounds, point, px, size}; + + use super::in_view; + + #[test] + fn a_caret_outside_the_field_hides() { + let field = Bounds::new(point(px(100.0), px(100.0)), size(px(200.0), px(60.0))); + let caret = |x: f32, y: f32| Bounds::new(point(px(x), px(y)), size(px(0.0), px(18.0))); + assert!(in_view(field, caret(150.0, 110.0))); + assert!(!in_view(field, caret(150.0, 200.0)), "below"); + assert!( + !in_view(field, caret(40.0, 110.0)), + "scrolled off to the left" + ); + assert!( + !in_view(field, caret(320.0, 110.0)), + "scrolled off to the right" + ); + assert!(in_view(field, caret(100.0, 110.0)), "at the left edge"); + } +} diff --git a/src/collab/share.rs b/src/collab/share.rs index f5f23f9..292b635 100644 --- a/src/collab/share.rs +++ b/src/collab/share.rs @@ -1,4 +1,4 @@ -use std::{path::Path, rc::Rc}; +use std::rc::Rc; use gpui::{ AnyElement, App, ElementId, FontWeight, IntoElement, ParentElement, RenderOnce, SharedString, @@ -8,6 +8,7 @@ use gpui::{ use crate::{ buttons::{Button, ButtonVariant, CopyButton, IconButton}, data_display::Avatar, + documents::source, forms::{Choice, OnValues, Pick, Run, Select, TagInput, is_email}, overlays::Dialog, primitives::{Icon, IconName}, @@ -31,6 +32,15 @@ const ROLES: [(Role, &str, &str); 3] = [ type OnRole = Rc; +/// The words for what `role` may do. +fn role_label(role: Role) -> &'static str { + let (_, _, label) = ROLES + .into_iter() + .find(|(listed, ..)| *listed == role) + .expect("every role is listed"); + label +} + /// Picks what someone may do: view, comment or edit. #[derive(IntoElement)] pub struct PermissionSelect { @@ -136,7 +146,7 @@ impl RenderOnce for AccessList { let id = |what: &str| (self.id.clone(), format!("{what}-{ix}")); let avatar = Avatar::new(id("avatar"), member.name.clone()).size(AvatarSize::Sm); let avatar = match &member.picture { - Some(picture) => avatar.image(Path::new(picture.as_ref())), + Some(picture) => avatar.image(source(picture)), None => avatar, }; let access: AnyElement = if member.owner { @@ -152,13 +162,20 @@ impl RenderOnce for AccessList { .flex() .items_center() .gap_1() - .children(on_role.map(|on_role| { - PermissionSelect::new( + .child(match on_role { + Some(on_role) => PermissionSelect::new( id("role"), member.role, move |role, window, cx| on_role(ix, role, window, cx), ) - })) + .into_any_element(), + None => div() + .px_2() + .text_size(theme.text_size(TextSize::Sm)) + .text_color(colors.fg_muted) + .child(role_label(member.role)) + .into_any_element(), + }) .children(on_remove.map(|on_remove| { IconButton::new(id("remove"), IconName::X) .variant(ButtonVariant::Ghost) diff --git a/src/documents/mod.rs b/src/documents/mod.rs index 78c7019..c8a35c2 100644 --- a/src/documents/mod.rs +++ b/src/documents/mod.rs @@ -25,6 +25,7 @@ mod viewer; use gpui::App; +pub(crate) use blocks::source; pub use blocks::{Align, BlockData, BlockEditor, BlockEvent, BlockKind, Media}; pub use counts::{ReadingTime, WordCount}; pub use epub::{Chapter, EpubReader};