From cae22f17aae0062d219158f2fae9c0d06680fb02 Mon Sep 17 00:00:00 2001 From: Albert Reig Date: Tue, 8 Sep 2026 22:20:30 -0600 Subject: [PATCH 1/6] TUI Calendar: open a read-only event card on Enter In the calendar's Day and Week views Enter did nothing on a highlighted event, and inside a Year cell it did nothing either, while the content help bar advertised "enter open" the whole time (the generic rowContent binding, live in Mail and dead here). The only way to see an event's notes, location, link or guests was to open the edit form with `e`. Enter now opens a read-only detail card over the grid, the way Contacts opens a contact on Enter and leaves `e` for editing. The card is built from the selected Recording alone -- the grid read already carries Notes, Location, Link and Attendees -- so nothing is fetched. From the card `o` opens the link, `e` swaps in the edit form on the same event, esc/q closes it, and the arrows and page keys scroll the notes. The card is an inputCapturer, so it handles esc itself and the help bar shows its keys instead of the generic "enter open". Two safeguards on what the card shows and does: - `o` only hands an http/https link to the OS launcher. Event links are server data and the edit form accepts any URI with a host, so a shared event could carry a file:// path or an application scheme; those are shown on the card but not opened, and `o` is not offered for them. - The when-and-where line shows the reader's local clock, the same conversion Recording.Starts/Ends and the grid make, rather than labelling a converted time with the event's original zone. The calendar name is sanitized like every other view of server metadata. Year view keeps its two stages: Enter steps into a cell, and only once inside does it open the selected event. Fixes #418 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C6Y2KeiHB6QNWmHdgqbzD1 --- docs/tui.md | 6 +- internal/tui/calendar.go | 103 ++++++++++++++- internal/tui/calendar_test.go | 231 ++++++++++++++++++++++++++++++++++ internal/tui/event_detail.go | 220 ++++++++++++++++++++++++++++++++ 4 files changed, 556 insertions(+), 4 deletions(-) create mode 100644 internal/tui/event_detail.go diff --git a/docs/tui.md b/docs/tui.md index d1ada41b..d2fb021a 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -131,6 +131,10 @@ Press Shift+O to open Contacts. Use Enter to view a contact, `a` to add, `e` to ## Calendar -Press Shift+C to open Calendar, then `c` to manage time track categories. Create a category with `n`, rename the selected category with Enter or `r`, and press `x` twice to delete it. Time tracks in a deleted category become uncategorized. +Press Shift+C to open Calendar. `1`, `2` and `3` switch between the day, week and year spans, `p` and `n` step back and forward, and `t` returns to today. The arrows walk what the span is made of — events on the day, days then events on the week, cells then a cell's events on the year (Enter steps into a cell, Escape steps back out). + +With an event picked out, Enter opens a read-only card showing what it carries — when and where it is, the link to join it, the guest list and the notes. From the card, `o` opens the link in your browser, `e` switches to the edit form on the same event, and Escape or `q` closes it. `a` creates an event, `e` edits the selected one, and `x` twice deletes it. + +Press `c` to manage time track categories. Create a category with `n`, rename the selected category with Enter or `r`, and press `x` twice to delete it. Time tracks in a deleted category become uncategorized. In Calendar, press `a` to create a habit. Habits visible in the current calendar range can be selected with `[` and `]`, edited with `e`, and deleted by pressing `x` twice. Habit forms use Tab to move between fields and Ctrl+S to save. diff --git a/internal/tui/calendar.go b/internal/tui/calendar.go index 77ed6a75..b5aba64f 100644 --- a/internal/tui/calendar.go +++ b/internal/tui/calendar.go @@ -471,6 +471,11 @@ type calendarView struct { // settings is the open calendar settings form, standing over the calendar. settings *calendarSettingsForm + // detail is the read-only card Enter opens over a selected event — everything the event + // carries, laid out to be read. It never stands with the event form: e closes it and + // opens the form on the same event. + detail *eventDetail + timeTrack *timeTrackMenu trackedTime *trackedTimeScreen // trackedTimeForm is the open edit form, standing over the tracked time screen. @@ -840,6 +845,11 @@ func (v *calendarView) View() string { frame := modalFrame(v.settings.title(), v.settings.view(), v.vc.width) view = overlayModal(view, frame, v.vc.width, v.vc.height) } + // The detail card stands over the grid like the event form does, and never with it: e + // closes the card and opens the form on the same event. + if v.detail != nil { + view = overlayModal(view, v.detail.view(), v.vc.width, v.vc.height) + } return view } @@ -867,6 +877,9 @@ func (v *calendarView) todosFooterHeight() int { } func (v *calendarView) HelpBindings() []helpBinding { + if v.detail != nil { + return v.detail.helpBindings() + } if v.settings != nil { return v.settings.helpBindings() } @@ -1034,6 +1047,25 @@ func (v *calendarView) handleContentKey(msg tea.KeyPressMsg) tea.Cmd { } return cmd } + // The event detail card takes every key while it is up — it is an inputCapturer, so the + // model routes esc here rather than through CancelPendingDetail. esc and q close it, o opens + // the link, e trades the card for the form on the same event, and anything else scrolls the + // notes or does nothing. + if v.detail != nil { + switch msg.String() { + case "esc", "q": + v.detail = nil + return nil + case "o": + return v.openEventLink() + case "e": + event := v.detail.event + v.detail = nil + return v.startEventForm(eventFormEdit, event) + } + return v.detail.update(msg) + } + if v.requests.kind == calendarRequestMutation { return nil } @@ -1130,6 +1162,9 @@ func (v *calendarView) handleContentKey(msg tea.KeyPressMsg) tea.Cmd { // the arrows move between cells, enter steps into one, and only then do ↑ and ↓ belong to that // day's events. esc steps back out. Without the two stages ↑ and ↓ would have to be both a // week's worth of movement and an event's, and a year of cells has no way to show which. +// +// enter opens the selected event's detail card wherever one is picked out — on the day, on the +// week, and inside a year cell. On the year with no cell open it is the step into the cell. func (v *calendarView) handleArrowKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { key := msg.String() @@ -1144,6 +1179,9 @@ func (v *calendarView) handleArrowKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { return v.crossTheDay(-1), true case "down": return v.crossTheDay(1), true + case "enter": + v.openEventDetail() + return nil, true } case viewWeek: switch key { @@ -1155,6 +1193,9 @@ func (v *calendarView) handleArrowKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { return v.moveSelection(-1), true case "down": return v.moveSelection(1), true + case "enter": + v.openEventDetail() + return nil, true } case viewYear: switch key { @@ -1173,6 +1214,10 @@ func (v *calendarView) handleArrowKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { } return v.moveCursorDay(7), true case "enter": + if v.inYearCell { + v.openEventDetail() + return nil, true + } v.enterYearCell() return nil, true } @@ -1269,7 +1314,8 @@ func (v *calendarView) leaveYearCell() { // CancelPendingDetail is how esc reaches a year cell. The model reads esc before a view sees a // key, and only offers it on through here — so stepping out of a cell is the same seam a mail -// thread's read is cancelled through, rather than a key the calendar handles itself. +// thread's read is cancelled through, rather than a key the calendar handles itself. The event +// card does not come through here: it is an inputCapturer, so the model hands it esc directly. func (v *calendarView) CancelPendingDetail() bool { if !v.inYearCell { return false @@ -1278,6 +1324,51 @@ func (v *calendarView) CancelPendingDetail() bool { return true } +// openEventDetail is Enter on the grid: the read-only card over whatever event the arrows have +// walked to. There is nothing to fetch — the grid read already carries the notes, the link and +// the guest list — so the card is built straight from the selected recording, and Enter with +// nothing picked out does nothing. +func (v *calendarView) openEventDetail() { + event, ok := v.selectedRecording() + if !ok { + return + } + v.detail = newEventDetail(event, v.calendarName(event.CalendarID), v.use24Hour, v.vc.styles, v.vc.width, v.vc.height) +} + +// openEventLink hands the card's event link to the same launcher that opens an attachment — +// xdg-open, open, the Windows handler. Only an http/https link is handed over (openableLink), +// so a shared event's file:// path or application scheme cannot invoke a local handler; a +// launcher missing from PATH says so in a toast rather than the key seeming dead. +func (v *calendarView) openEventLink() tea.Cmd { + if v.detail == nil { + return nil + } + link, ok := v.detail.openableLink() + if !ok { + return nil + } + if v.vc.openAttachment == nil { + return nil + } + if err := v.vc.openAttachment(link); err != nil { + return notifyError("Could not open the link", err) + } + return notify("Opening the link…") +} + +// calendarName is the event's calendar by name, for the detail card. The personal calendar and +// any calendar the reader is not a member of are not in the list, and get no name rather than +// a wrong one. +func (v *calendarView) calendarName(id int64) string { + for _, calendar := range v.calendars { + if calendar.ID == id { + return calendar.Name + } + } + return "" +} + // handleHabitPickerKey gives the open picker every key: managing a habit is what the // modal is for, so a is a new habit here rather than whatever a means outside it. // handleCalendarPickerKey gives the open picker every key. The picker stays open across a @@ -1595,7 +1686,7 @@ func (v *calendarView) Loading() bool { } func (v *calendarView) CapturingInput() bool { return v.timeTrack != nil || v.trackedTime != nil || v.timeTrackCategories != nil || - v.habitForm != nil || v.eventForm != nil || v.settings != nil || + v.habitForm != nil || v.eventForm != nil || v.settings != nil || v.detail != nil || v.habitPicker != nil || v.todoPicker != nil || v.calendarPicker != nil } @@ -1620,11 +1711,14 @@ func (v *calendarView) refreshLive() (tea.Cmd, bool) { } // Restyle re-renders the day/week/year grid, which caches styled output in its -// viewport. The recording detail is plain text and needs nothing. +// viewport, and the event card, which caches its own. func (v *calendarView) Restyle() { if v.trackedTime != nil { v.trackedTime.rebuild() } + if v.detail != nil { + v.detail.restyle(v.vc.styles) + } v.rebuildKeepingScroll() } @@ -1656,6 +1750,9 @@ func (v *calendarView) Resize(width, height int) { if v.settings != nil { v.settings.resize(width, height) } + if v.detail != nil { + v.detail.resize(width, height) + } v.rebuildView() } diff --git a/internal/tui/calendar_test.go b/internal/tui/calendar_test.go index 3e50939a..644e2150 100644 --- a/internal/tui/calendar_test.go +++ b/internal/tui/calendar_test.go @@ -644,6 +644,199 @@ func TestARepeatingEventsOwnDayCanBeSelected(t *testing.T) { } } +// cardDay is a calendar on one day holding a single event with every trimming — a location, a +// link, guests, notes and a weekly recurrence — so the read-only card can be checked in full. +func cardDay(t *testing.T) *calendarView { + t.Helper() + v := newCalendarView(testVC()) + v.Resize(100, 30) + v.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + v.Update(calendarsLoadedMsg{calendars: testCalendars()}) + v.Update(recordingsLoadedMsg{requestResult: currentRequest(v), recordings: []Recording{ + {ID: 5, Title: "Roadmap review", Type: "Calendar::Event", CalendarID: 10, + StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00"), + Location: "Sala 2", Link: "https://meet.example.com/roadmap", + Attendees: []string{"ana@example.com", "luis@example.com"}, + Notes: "Bring the September report", Recurring: true, RepeatKind: "every_week"}, + }}) + return v +} + +// Enter opens a read-only card over whatever event the arrows have walked to — the same offer +// the help bar makes on every other content list. It is built from the selection alone: the +// grid read already carries the notes, the link and the guests. +func TestEnterOpensTheReadOnlyEventCard(t *testing.T) { + v := cardDay(t) + + // With nothing picked out there is nothing to open. + if cmd := v.HandleContentKey(keyPress("enter")); cmd != nil || v.detail != nil { + t.Fatal("enter opened a card with nothing selected") + } + + v.HandleContentKey(keyPress("right")) + if v.selectedEvent != "5" { + t.Fatalf("→ selected %q", v.selectedEvent) + } + v.HandleContentKey(keyPress("enter")) + if v.detail == nil { + t.Fatal("enter did not open the event card") + } + + card := stripANSI(v.detail.view()) + for _, want := range []string{ + "Roadmap review", "Sala 2", "https://meet.example.com/roadmap", + "ana@example.com", "Bring the September report", "every week", "Design Team", + } { + if !strings.Contains(card, want) { + t.Errorf("the card does not show %q:\n%s", want, card) + } + } + + // The card holds every key: a span number does not switch the view behind it. + v.HandleContentKey(keyPress("3")) + if v.viewMode != viewDay || v.detail == nil { + t.Errorf("a key fell through the card: viewMode=%v open=%v", v.viewMode, v.detail != nil) + } + + if !hasBinding(v.HelpBindings(), "o") || !hasBinding(v.HelpBindings(), "e") { + t.Errorf("the card's help bar = %+v, want o and e", v.HelpBindings()) + } + + // esc closes it, and so does q. + v.HandleContentKey(keyPress("esc")) + if v.detail != nil { + t.Fatal("esc did not close the card") + } + v.HandleContentKey(keyPress("right")) + v.HandleContentKey(keyPress("enter")) + v.HandleContentKey(keyPress("q")) + if v.detail != nil { + t.Error("q did not close the card") + } +} + +// o on the card opens the link through the same launcher an attachment uses. +func TestTheEventCardOpensTheLink(t *testing.T) { + var opened []string + vc := testVC() + vc.openAttachment = func(target string) error { + opened = append(opened, target) + return nil + } + + v := newCalendarView(vc) + v.Resize(100, 30) + v.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + v.Update(calendarsLoadedMsg{calendars: testCalendars()}) + v.Update(recordingsLoadedMsg{requestResult: currentRequest(v), recordings: []Recording{ + {ID: 6, Title: "Sync", Type: "Calendar::Event", + StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00"), + Link: "https://meet.example.com/sync"}, + }}) + v.HandleContentKey(keyPress("right")) + v.HandleContentKey(keyPress("enter")) + + if cmd := v.HandleContentKey(keyPress("o")); cmd == nil { + t.Fatal("o said nothing") + } + if len(opened) != 1 || opened[0] != "https://meet.example.com/sync" { + t.Fatalf("o opened %v, want the event link", opened) + } + if v.detail == nil { + t.Error("o closed the card") + } +} + +// Event links are server data and the edit form takes any URI with a host, so a shared event +// could carry a non-web scheme. The card shows it but never hands it to the OS launcher, and +// does not offer o for it. +func TestTheEventCardWillNotOpenANonWebLink(t *testing.T) { + var opened []string + vc := testVC() + vc.openAttachment = func(target string) error { + opened = append(opened, target) + return nil + } + + v := newCalendarView(vc) + v.Resize(100, 30) + v.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + v.Update(calendarsLoadedMsg{calendars: testCalendars()}) + v.Update(recordingsLoadedMsg{requestResult: currentRequest(v), recordings: []Recording{ + {ID: 6, Title: "Sync", Type: "Calendar::Event", + StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00"), + Link: "file:///etc/passwd"}, + }}) + v.HandleContentKey(keyPress("right")) + v.HandleContentKey(keyPress("enter")) + + if hasBinding(v.HelpBindings(), "o") { + t.Error("the card offers o for a non-web link") + } + v.HandleContentKey(keyPress("o")) + if len(opened) != 0 { + t.Fatalf("o handed %v to the launcher", opened) + } + if !strings.Contains(stripANSI(v.detail.view()), "file:///etc/passwd") { + t.Error("the card hides the link instead of showing it") + } +} + +// The card is an inputCapturer, so the model routes every key to it -- including esc, which +// never reaches CancelPendingDetail while it is open. Regression test that esc closes the card +// through the full model rather than being swallowed by the notes viewport. +func TestModelClosesTheEventCardOnEscape(t *testing.T) { + m := sizedModel() + m.loading = false + m.section = sectionCalendar + m.activeView = m.calendarView + m.calendarView.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + m.calendarView.Update(calendarsLoadedMsg{calendars: testCalendars()}) + m.calendarView.Update(recordingsLoadedMsg{ + requestResult: currentRequest(m.calendarView), + recordings: []Recording{ + {ID: 5, Title: "Roadmap review", Type: "Calendar::Event", CalendarID: 10, + StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00"), + Notes: "Bring the September report"}, + }, + }) + + step := func(key string) { + t.Helper() + updated, _ := m.Update(keyPress(key)) + m = updated.(model) + } + + step("right") + step("enter") + if m.calendarView.detail == nil { + t.Fatal("enter did not open the card through the model") + } + step("esc") + if m.calendarView.detail != nil { + t.Fatal("the model did not close the card on esc") + } +} + +// e trades the card for the edit form on the same event, so the card is where an edit starts +// rather than a dead end. +func TestEEditsFromTheEventCard(t *testing.T) { + v := cardDay(t) + v.HandleContentKey(keyPress("right")) + v.HandleContentKey(keyPress("enter")) + + v.HandleContentKey(keyPress("e")) + if v.detail != nil { + t.Error("e left the card open") + } + if v.eventForm == nil || v.eventForm.mode != eventFormEdit { + t.Fatal("e did not open the edit form on the card's event") + } + if v.editing.ID != 5 { + t.Errorf("the form is editing %d, want the card's event", v.editing.ID) + } +} + // On the year, b manages habits but does not keep them. A year read carries no recordings, so // nothing on that screen knows what was kept on the day the cursor is on — and a ring drawn // empty there would be answering a question nobody asked the server. @@ -1100,6 +1293,44 @@ func TestYearArrowsMoveCellsUntilOneIsOpened(t *testing.T) { } } +// Inside a year cell enter opens the selected event's card, the same as on the day and the +// week. esc then closes the card and leaves the cell standing, so leaving the year takes two. +func TestEnterOpensTheEventCardInsideAYearCell(t *testing.T) { + v := newCalendarView(testVC()) + v.Resize(100, 30) + v.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + v.viewMode = viewYear + v.Update(calendarsLoadedMsg{calendars: testCalendars()}) + v.Update(yearLoadedMsg{requestResult: currentRequest(v), year: CalendarYear{ + SpannedEvents: []Recording{ + {ID: 7, Title: "Off to Split", AllDay: true, Type: "Calendar::Event", + StartsAt: at("2026-08-21T00:00:00Z"), EndsAt: at("2026-08-21T00:00:00Z")}, + }, + }}) + + v.HandleContentKey(keyPress("right")) + v.HandleContentKey(keyPress("enter")) // step into the cell + v.HandleContentKey(keyPress("enter")) // open the card + if v.detail == nil { + t.Fatal("enter in a year cell did not open the card") + } + if !strings.Contains(stripANSI(v.detail.view()), "Off to Split") { + t.Errorf("the card is not the selected event:\n%s", stripANSI(v.detail.view())) + } + + v.HandleContentKey(keyPress("esc")) + if v.detail != nil { + t.Fatal("esc did not close the card") + } + if !v.inYearCell { + t.Error("closing the card also left the cell") + } + // A second esc, now through the model's seam, steps out of the cell. + if !v.CancelPendingDetail() || v.inYearCell { + t.Error("esc did not step out of the cell once the card was closed") + } +} + // The all-day band is at the foot of the whole week, but the events in it belong to days, so ↑ // and ↓ reach the cursor day's own — and the band draws it as selected once they have. func TestWeekReachesTheAllDayBand(t *testing.T) { diff --git a/internal/tui/event_detail.go b/internal/tui/event_detail.go new file mode 100644 index 00000000..41f30102 --- /dev/null +++ b/internal/tui/event_detail.go @@ -0,0 +1,220 @@ +package tui + +import ( + "fmt" + "net/url" + "strings" + + "charm.land/bubbles/v2/viewport" + tea "charm.land/bubbletea/v2" + + "github.com/basecamp/hey-cli/internal/terminal" +) + +// eventDetail is the read-only card Enter opens over a selected event: what the event carries — +// when and where it is, the link to join it, who is coming, the notes — laid out to be read +// rather than typed into. e steps through to the edit form and o opens the link; esc closes it. +// +// It holds the Recording it was opened on rather than an id: the grid read already carries the +// notes, the location, the link and the guest list (see the fields on Recording, kept for +// exactly this reason), so there is nothing here to fetch. +type eventDetail struct { + event Recording + calendar string // the event's calendar by name, or "" for the personal one + use24 bool + + body viewport.Model + styles styles + width int + height int +} + +func newEventDetail(event Recording, calendar string, use24 bool, styles styles, width, height int) *eventDetail { + d := &eventDetail{ + event: event, + calendar: calendar, + use24: use24, + styles: styles, + body: viewport.New(viewport.WithWidth(0), viewport.WithHeight(0)), + } + d.resize(width, height) + return d +} + +// resize refits the card to the screen. The body is capped at what a modal has room for and +// scrolls past that, so a long set of notes does not push the frame off either end. +func (d *eventDetail) resize(width, height int) { + d.width, d.height = width, height + content := d.content() + d.body.SetWidth(modalContentWidth(width)) + d.body.SetHeight(min(lineCount(content), modalContentRows(height))) + offset := d.body.YOffset() + d.body.SetContent(content) + d.body.SetYOffset(offset) +} + +// restyle re-renders the card with a new palette, keeping the reader's place in the notes. +func (d *eventDetail) restyle(styles styles) { + d.styles = styles + d.resize(d.width, d.height) +} + +func (d *eventDetail) update(msg tea.Msg) tea.Cmd { + var cmd tea.Cmd + d.body, cmd = d.body.Update(msg) + return cmd +} + +func (d *eventDetail) view() string { + return modalFrame(d.title(), d.body.View(), d.width) +} + +// title is the event's own name, or the parent's for a recording that has none of its own — +// a countdown carries "10 days before" as a label and leans on the event above it for a name. +func (d *eventDetail) title() string { + if d.event.Title != "" { + return terminal.SanitizeLine(d.event.Title) + } + if d.event.ParentTitle != "" { + return terminal.SanitizeLine(d.event.ParentTitle) + } + return "Event" +} + +func (d *eventDetail) helpBindings() []helpBinding { + bindings := make([]helpBinding, 0, 3) + if _, ok := d.openableLink(); ok { + bindings = append(bindings, helpBinding{"o", "open link"}) + } + bindings = append(bindings, helpBinding{"e", "edit"}, helpBinding{"esc", "back"}) + return bindings +} + +// openableLink is the event's link when it is a web address the OS launcher should be handed: +// http or https. Event links are server data and the edit form takes any URI with a host, so a +// shared event could carry a file:// path or an application scheme — those are shown on the +// card but not opened. +func (d *eventDetail) openableLink() (string, bool) { + link := strings.TrimSpace(d.event.Link) + if link == "" { + return "", false + } + parsed, err := url.Parse(link) + if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") { + return "", false + } + return link, true +} + +// content is the card's body: the when-and-where up top, then whichever of the optional +// fields the event actually has, then the notes. A field with nothing in it is left out +// rather than shown empty, the way the web app's event popover does. +func (d *eventDetail) content() string { + var b strings.Builder + + b.WriteString(d.styles.entryDate.Render(d.when()) + "\n") + if label := repeatFrequencyLabel(d.event.RepeatKind); d.event.Recurring && label != "" { + b.WriteString(styleMuted.Render("Repeats "+label) + "\n") + } + + rows := [][2]string{ + {"Calendar", terminal.SanitizeLine(d.calendar)}, + {"Location", terminal.SanitizeLine(d.event.Location)}, + {"Link", terminal.SanitizeLine(d.event.Link)}, + {"Guests", d.guests()}, + } + wrote := false + for _, row := range rows { + if row[1] == "" { + continue + } + if !wrote { + b.WriteString("\n") + wrote = true + } + fmt.Fprintf(&b, "%s %s\n", d.styles.entryFrom.Render(fmt.Sprintf("%-8s", row[0])), row[1]) + } + + if notes := strings.TrimRight(d.event.Notes, "\n"); strings.TrimSpace(notes) != "" { + b.WriteString("\n" + d.styles.entryFrom.Render("Notes") + "\n") + for _, line := range wrapParagraphs(terminal.Sanitize(notes), modalContentWidth(d.width)) { + b.WriteString(line + "\n") + } + } + + return strings.TrimRight(b.String(), "\n") +} + +// when is the one line that says the whole of an event's timing: the day and the hours, on the +// reader's own clock — the same conversion Recording.Starts and Ends make, and the same one the +// grid draws by, so a zoned event is not relabelled here with a zone its shown time is not in. +// An all-day event says so instead of a clock time; one that runs past midnight names the day +// it ends on. +func (d *eventDetail) when() string { + starts := d.event.Starts() + if starts.IsZero() { + return "When unknown" + } + ends := d.event.Ends() + + if d.event.AllDay { + if !ends.IsZero() && ends.After(starts) { + return starts.Format("Monday, January 2") + " – " + ends.Format("Monday, January 2") + " · all day" + } + return starts.Format("Monday, January 2") + " · all day" + } + + line := starts.Format("Monday, January 2") + " · " + clockTime(starts, d.use24) + switch { + case ends.IsZero() || !ends.After(starts): + case sameDay(starts, ends): + line += "–" + clockTime(ends, d.use24) + default: + line += " – " + ends.Format("Monday, January 2") + " · " + clockTime(ends, d.use24) + } + return line +} + +func (d *eventDetail) guests() string { + if len(d.event.Attendees) == 0 { + return "" + } + clean := make([]string, 0, len(d.event.Attendees)) + for _, attendee := range d.event.Attendees { + if trimmed := terminal.SanitizeLine(attendee); trimmed != "" { + clean = append(clean, trimmed) + } + } + return strings.Join(clean, ", ") +} + +// repeatFrequencyLabel turns the schedule kind HEY serves — "every_week" and the like — back +// into the words the repeat picker offers, so the card and the form say a recurrence the same +// way. An unknown kind gets no line rather than a raw token. +func repeatFrequencyLabel(kind string) string { + if kind == "" { + return "" + } + for _, preset := range eventRepeatPresets { + if string(preset.frequency) == kind { + return preset.label + } + } + return "" +} + +// wrapParagraphs wraps each line to width while keeping the blank lines between paragraphs, +// which wrapText on its own drops — notes are written with those breaks and read worse without. +func wrapParagraphs(text string, width int) []string { + var lines []string + for _, paragraph := range strings.Split(text, "\n") { + if strings.TrimSpace(paragraph) == "" { + lines = append(lines, "") + continue + } + lines = append(lines, wrapText(paragraph, width)...) + } + return lines +} + +func lineCount(s string) int { return strings.Count(s, "\n") + 1 } From 9afb68a74434b2307af294bd5c4d7dbaa4ea301e Mon Sep 17 00:00:00 2001 From: albertreig Date: Sun, 13 Sep 2026 20:39:57 +0000 Subject: [PATCH 2/6] Fix Copilot review: link validation, modal wrapping, all-day dates, Week-view test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - openableLink: require non-empty host and compare scheme case-insensitively, so hostless URIs (https:roadmap, https://) and uppercase schemes are handled. - content() rows: wrap long values (URL, attendee list) to available modal width so no row makes the modal wider than the terminal. - wrapText: hard-wrap tokens that exceed maxWidth so a long URL in notes cannot overflow the card viewport. - when() all-day: collapse same-day ends and subtract exclusive midnight boundary so Aug 21 00:00–23:59 and Aug 21 00:00–Aug 22 00:00 both show as a single day. - Add TestEnterOpensTheEventCardOnWeekView, TestAllDayEvent*, TestWrapText* and extend TestEventDetailOpensOnlyHostedWebLinks with new cases. --- internal/tui/event_detail.go | 41 ++++++++-- internal/tui/event_detail_test.go | 122 ++++++++++++++++++++++++++++++ internal/tui/styles.go | 26 ++++++- 3 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 internal/tui/event_detail_test.go diff --git a/internal/tui/event_detail.go b/internal/tui/event_detail.go index 41f30102..afb6b61e 100644 --- a/internal/tui/event_detail.go +++ b/internal/tui/event_detail.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" "strings" + "time" "charm.land/bubbles/v2/viewport" tea "charm.land/bubbletea/v2" @@ -91,16 +92,24 @@ func (d *eventDetail) helpBindings() []helpBinding { } // openableLink is the event's link when it is a web address the OS launcher should be handed: -// http or https. Event links are server data and the edit form takes any URI with a host, so a -// shared event could carry a file:// path or an application scheme — those are shown on the -// card but not opened. +// http or https, with a non-empty host. Event links are server data and the edit form takes any +// URI with a host, so a shared event could carry a file:// path, an application scheme, or a +// hostless URI like "https:roadmap" — those are shown on the card but never opened. +// The scheme comparison is case-insensitive so "HTTPS://…" is treated the same as "https://…". func (d *eventDetail) openableLink() (string, bool) { link := strings.TrimSpace(d.event.Link) if link == "" { return "", false } parsed, err := url.Parse(link) - if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") { + if err != nil { + return "", false + } + scheme := strings.ToLower(parsed.Scheme) + if scheme != "http" && scheme != "https" { + return "", false + } + if parsed.Host == "" { return "", false } return link, true @@ -123,6 +132,9 @@ func (d *eventDetail) content() string { {"Link", terminal.SanitizeLine(d.event.Link)}, {"Guests", d.guests()}, } + // The label column is 8 chars + 2 spaces of padding; what remains is for the value. + labelWidth := 10 + valueWidth := max(modalContentWidth(d.width)-labelWidth, 1) wrote := false for _, row := range rows { if row[1] == "" { @@ -132,7 +144,14 @@ func (d *eventDetail) content() string { b.WriteString("\n") wrote = true } - fmt.Fprintf(&b, "%s %s\n", d.styles.entryFrom.Render(fmt.Sprintf("%-8s", row[0])), row[1]) + // Wrap long values so no row makes the modal wider than the terminal. + for i, valueLine := range wrapText(row[1], valueWidth) { + if i == 0 { + fmt.Fprintf(&b, "%s %s\n", d.styles.entryFrom.Render(fmt.Sprintf("%-8s", row[0])), valueLine) + } else { + fmt.Fprintf(&b, "%s %s\n", strings.Repeat(" ", 8), valueLine) + } + } } if notes := strings.TrimRight(d.event.Notes, "\n"); strings.TrimSpace(notes) != "" { @@ -159,7 +178,17 @@ func (d *eventDetail) when() string { if d.event.AllDay { if !ends.IsZero() && ends.After(starts) { - return starts.Format("Monday, January 2") + " – " + ends.Format("Monday, January 2") + " · all day" + // Normalize same-day: if end is the same calendar day as start, it's a single-day event. + // Also handle exclusive midnight end: a midnight end belongs to the previous day. + endDay := ends + if ends.Hour() == 0 && ends.Minute() == 0 && ends.Second() == 0 { + // Exclusive midnight end — the event ends before this day begins. + endDay = ends.Add(-24 * time.Hour) + } + if sameDay(starts, endDay) { + return starts.Format("Monday, January 2") + " · all day" + } + return starts.Format("Monday, January 2") + " – " + endDay.Format("Monday, January 2") + " · all day" } return starts.Format("Monday, January 2") + " · all day" } diff --git a/internal/tui/event_detail_test.go b/internal/tui/event_detail_test.go new file mode 100644 index 00000000..8daa6d47 --- /dev/null +++ b/internal/tui/event_detail_test.go @@ -0,0 +1,122 @@ +package tui + +import ( + "strings" + "testing" + "time" +) + +func TestEventDetailOpensOnlyHostedWebLinks(t *testing.T) { + for _, test := range []struct { + name string + link string + want bool + }{ + {name: "http link", link: "http://meet.example.com/roadmap", want: true}, + {name: "HTTPS link (uppercase scheme)", link: "HTTPS://meet.example.com/roadmap", want: true}, + {name: "hostless HTTPS link", link: "https:roadmap", want: false}, + {name: "empty HTTPS host", link: "https://", want: false}, + {name: "file link", link: "file:///etc/passwd", want: false}, + {name: "application scheme", link: "zoomus://zoom.us/join/123", want: false}, + {name: "empty link", link: "", want: false}, + } { + t.Run(test.name, func(t *testing.T) { + detail := &eventDetail{event: Recording{Link: test.link}} + _, got := detail.openableLink() + if got != test.want { + t.Errorf("openableLink(%q) = %v, want %v", test.link, got, test.want) + } + }) + } +} + +// Enter on the week view opens the selected event's card — the same as on the day view. +func TestEnterOpensTheEventCardOnWeekView(t *testing.T) { + v := newCalendarView(testVC()) + v.Resize(100, 30) + v.viewMode = viewWeek + v.now = func() time.Time { return time.Date(2026, 8, 20, 9, 0, 0, 0, time.Local) } + v.Update(calendarsLoadedMsg{calendars: testCalendars()}) + v.Update(recordingsLoadedMsg{requestResult: currentRequest(v), recordings: []Recording{ + {ID: 8, Title: "Team sync", Type: "Calendar::Event", + StartsAt: atLocal("2026-08-20T10:00:00"), EndsAt: atLocal("2026-08-20T11:00:00")}, + }}) + + // In the week view ↑/↓ walk events within the current day. + v.HandleContentKey(keyPress("down")) + if v.selectedEvent != "8" { + t.Fatalf("down did not select the event in week view (selectedEvent=%q)", v.selectedEvent) + } + v.HandleContentKey(keyPress("enter")) + if v.detail == nil { + t.Fatal("enter on week view did not open the event card") + } + if !strings.Contains(stripANSI(v.detail.view()), "Team sync") { + t.Errorf("week-view card does not show the event title:\n%s", stripANSI(v.detail.view())) + } + v.HandleContentKey(keyPress("esc")) + if v.detail != nil { + t.Fatal("esc did not close the card on week view") + } +} + +// An all-day event with a same-day end should show as a single day, not a range. +func TestAllDayEventSameDayEndShowsAsSingleDay(t *testing.T) { + d := &eventDetail{event: Recording{ + AllDay: true, + StartsAt: at("2026-08-21T00:00:00Z"), + EndsAt: at("2026-08-21T23:59:59Z"), + }} + got := d.when() + if strings.Contains(got, "–") { + t.Errorf("same-day all-day event showed a range: %q", got) + } + if !strings.Contains(got, "all day") { + t.Errorf("all-day event missing 'all day': %q", got) + } +} + +// An all-day event whose end is exactly midnight of the next day (exclusive) should show as +// a single day, not "Aug 21 – Aug 22". +func TestAllDayEventExclusiveMidnightEndShowsAsSingleDay(t *testing.T) { + d := &eventDetail{event: Recording{ + AllDay: true, + StartsAt: at("2026-08-21T00:00:00Z"), + EndsAt: at("2026-08-22T00:00:00Z"), // exclusive midnight + }} + got := d.when() + if strings.Contains(got, "–") { + t.Errorf("exclusive-midnight all-day event showed a range: %q", got) + } + if !strings.Contains(got, "all day") { + t.Errorf("all-day event missing 'all day': %q", got) + } +} + +// A multi-day all-day event should still show the range. +func TestAllDayEventMultiDayShowsRange(t *testing.T) { + d := &eventDetail{event: Recording{ + AllDay: true, + StartsAt: at("2026-08-21T00:00:00Z"), + EndsAt: at("2026-08-23T00:00:00Z"), // exclusive — event covers Aug 21 & 22 + }} + got := d.when() + if !strings.Contains(got, "–") { + t.Errorf("multi-day all-day event did not show a range: %q", got) + } +} + +// wrapText must hard-wrap a single token that exceeds maxWidth. +func TestWrapTextHardWrapsLongTokens(t *testing.T) { + long := "https://meet.example.com/a-very-long-room-name-that-exceeds-the-modal-width" + lines := wrapText(long, 20) + for _, line := range lines { + if len(line) > 20 { + t.Errorf("wrapText produced a line longer than maxWidth: %q (len=%d)", line, len(line)) + } + } + rejoined := strings.Join(lines, "") + if rejoined != long { + t.Errorf("wrapText lost characters: got %q, want %q", rejoined, long) + } +} diff --git a/internal/tui/styles.go b/internal/tui/styles.go index f717d815..37e01c64 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -215,6 +215,8 @@ func errorView(errMsg string, width int) string { } // wrapText wraps a string to fit within maxWidth characters. +// Words longer than maxWidth are hard-wrapped at the boundary so that a single long token +// (like a URL) does not make the containing column wider than the terminal. func wrapText(s string, maxWidth int) []string { if maxWidth <= 0 { return []string{s} @@ -225,15 +227,31 @@ func wrapText(s string, maxWidth int) []string { } var lines []string - line := words[0] - for _, w := range words[1:] { - if len(line)+1+len(w) > maxWidth { + line := "" + for _, w := range words { + // Hard-wrap any word that alone exceeds maxWidth. + for len(w) > maxWidth { + if line != "" { + lines = append(lines, line) + line = "" + } + lines = append(lines, w[:maxWidth]) + w = w[maxWidth:] + } + if w == "" { + continue + } + if line == "" { + line = w + } else if len(line)+1+len(w) > maxWidth { lines = append(lines, line) line = w } else { line += " " + w } } - lines = append(lines, line) + if line != "" { + lines = append(lines, line) + } return lines } From ebea16c513cab612bdccec36e8ea8c96407ab792 Mon Sep 17 00:00:00 2001 From: albertreig Date: Sun, 13 Sep 2026 20:56:23 +0000 Subject: [PATCH 3/6] Fix Copilot review round 2: grapheme-safe wrapping and narrow-terminal when() line - wrapText: use displayWidth/fitGraphemes instead of byte slicing so hard-wrap never splits inside a rune or emoji sequence, and wide characters (emoji, CJK) are measured in terminal cells rather than bytes. - content(): wrap the when() line to modalContentWidth so a long weekday+time range does not overflow the modal on narrow terminals. - Add TestWrapTextSplitsAtGraphemeBoundaries and TestEventCardWhenLineWrapsOnNarrowTerminal. --- internal/tui/event_detail.go | 12 +++++++-- internal/tui/event_detail_test.go | 45 +++++++++++++++++++++++++++++-- internal/tui/styles.go | 28 +++++++++++++------ 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/internal/tui/event_detail.go b/internal/tui/event_detail.go index afb6b61e..72bb4c20 100644 --- a/internal/tui/event_detail.go +++ b/internal/tui/event_detail.go @@ -121,7 +121,15 @@ func (d *eventDetail) openableLink() (string, bool) { func (d *eventDetail) content() string { var b strings.Builder - b.WriteString(d.styles.entryDate.Render(d.when()) + "\n") + contentWidth := modalContentWidth(d.width) + // The when line can exceed contentWidth on narrow terminals; wrap it like the other rows. + for i, whenLine := range wrapText(d.when(), contentWidth) { + if i == 0 { + b.WriteString(d.styles.entryDate.Render(whenLine) + "\n") + } else { + b.WriteString(whenLine + "\n") + } + } if label := repeatFrequencyLabel(d.event.RepeatKind); d.event.Recurring && label != "" { b.WriteString(styleMuted.Render("Repeats "+label) + "\n") } @@ -134,7 +142,7 @@ func (d *eventDetail) content() string { } // The label column is 8 chars + 2 spaces of padding; what remains is for the value. labelWidth := 10 - valueWidth := max(modalContentWidth(d.width)-labelWidth, 1) + valueWidth := max(contentWidth-labelWidth, 1) wrote := false for _, row := range rows { if row[1] == "" { diff --git a/internal/tui/event_detail_test.go b/internal/tui/event_detail_test.go index 8daa6d47..7c792e76 100644 --- a/internal/tui/event_detail_test.go +++ b/internal/tui/event_detail_test.go @@ -4,6 +4,8 @@ import ( "strings" "testing" "time" + + "github.com/charmbracelet/x/ansi" ) func TestEventDetailOpensOnlyHostedWebLinks(t *testing.T) { @@ -111,8 +113,8 @@ func TestWrapTextHardWrapsLongTokens(t *testing.T) { long := "https://meet.example.com/a-very-long-room-name-that-exceeds-the-modal-width" lines := wrapText(long, 20) for _, line := range lines { - if len(line) > 20 { - t.Errorf("wrapText produced a line longer than maxWidth: %q (len=%d)", line, len(line)) + if displayWidth(line) > 20 { + t.Errorf("wrapText produced a line wider than maxWidth: %q (width=%d)", line, displayWidth(line)) } } rejoined := strings.Join(lines, "") @@ -120,3 +122,42 @@ func TestWrapTextHardWrapsLongTokens(t *testing.T) { t.Errorf("wrapText lost characters: got %q, want %q", rejoined, long) } } + +// wrapText must split at grapheme boundaries, never inside a rune or emoji sequence. +func TestWrapTextSplitsAtGraphemeBoundaries(t *testing.T) { + // A string of 5 two-cell emoji, each 2 cells wide, total 10 cells. + emoji := "🎉🎊🎈🎁🎀" + lines := wrapText(emoji, 4) // 4-cell budget: fits two emoji per line + for _, line := range lines { + w := displayWidth(line) + if w > 4 { + t.Errorf("wrapText produced a line wider than maxWidth: %q (width=%d)", line, w) + } + // Verify each line decodes as valid UTF-8 grapheme clusters. + for rest := line; rest != ""; { + cluster, _ := ansi.FirstGraphemeCluster(rest, ansi.GraphemeWidth) + if cluster == "" { + t.Errorf("wrapText produced invalid UTF-8 in %q", line) + break + } + rest = rest[len(cluster):] + } + } +} + +// On a narrow terminal the when() line must not exceed the modal content width. +func TestEventCardWhenLineWrapsOnNarrowTerminal(t *testing.T) { + d := &eventDetail{ + event: Recording{StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00")}, + styles: testVC().styles, + width: 40, // narrow terminal + height: 20, + } + content := d.content() + for _, line := range strings.Split(content, "\n") { + stripped := ansi.Strip(line) + if displayWidth(stripped) > modalContentWidth(40) { + t.Errorf("content line exceeds modal content width on narrow terminal: %q (width=%d)", stripped, displayWidth(stripped)) + } + } +} diff --git a/internal/tui/styles.go b/internal/tui/styles.go index 37e01c64..1a5a783f 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -214,9 +214,10 @@ func errorView(errMsg string, width int) string { return b.String() } -// wrapText wraps a string to fit within maxWidth characters. -// Words longer than maxWidth are hard-wrapped at the boundary so that a single long token -// (like a URL) does not make the containing column wider than the terminal. +// wrapText wraps a string to fit within maxWidth terminal cells. +// Words wider than maxWidth are hard-wrapped at grapheme boundaries so that a single long +// token (like a URL or an emoji-heavy value) never makes the column wider than the terminal +// and is never split inside a rune or grapheme cluster. func wrapText(s string, maxWidth int) []string { if maxWidth <= 0 { return []string{s} @@ -228,26 +229,37 @@ func wrapText(s string, maxWidth int) []string { var lines []string line := "" + lineWidth := 0 for _, w := range words { - // Hard-wrap any word that alone exceeds maxWidth. - for len(w) > maxWidth { + // Hard-wrap any word whose display width alone exceeds maxWidth, advancing by + // whole grapheme clusters so we never split inside a rune or emoji sequence. + for displayWidth(w) > maxWidth { if line != "" { lines = append(lines, line) line = "" + lineWidth = 0 } - lines = append(lines, w[:maxWidth]) - w = w[maxWidth:] + chunk := fitGraphemes(w, maxWidth) + if chunk == "" { + break // single cluster wider than maxWidth — emit it whole to avoid infinite loop + } + lines = append(lines, chunk) + w = w[len(chunk):] } if w == "" { continue } + wWidth := displayWidth(w) if line == "" { line = w - } else if len(line)+1+len(w) > maxWidth { + lineWidth = wWidth + } else if lineWidth+1+wWidth > maxWidth { lines = append(lines, line) line = w + lineWidth = wWidth } else { line += " " + w + lineWidth += 1 + wWidth } } if line != "" { From ff27b8f9fa26f41d539d9018b3b69ee9fcc550e4 Mon Sep 17 00:00:00 2001 From: albertreig Date: Mon, 14 Sep 2026 15:11:50 +0000 Subject: [PATCH 4/6] Fix Copilot review round 3: safe edit transition and cell-accurate errorView - calendar.go: only clear the detail card after confirming v.eventForm was created; if startEventForm finds no fileable calendars it returns an error notice without setting v.eventForm, so a failed edit transition now leaves the read-only card visible instead of leaving the user with nothing. - styles.go: use displayWidth instead of len() in errorView so the box width is measured in terminal cells, consistent with wrapText's new contract; a line of wide characters (emoji, CJK) no longer expands the error box past the terminal. --- internal/tui/calendar.go | 10 ++++++++-- internal/tui/styles.go | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/internal/tui/calendar.go b/internal/tui/calendar.go index b5aba64f..842a797c 100644 --- a/internal/tui/calendar.go +++ b/internal/tui/calendar.go @@ -1060,8 +1060,14 @@ func (v *calendarView) handleContentKey(msg tea.KeyPressMsg) tea.Cmd { return v.openEventLink() case "e": event := v.detail.event - v.detail = nil - return v.startEventForm(eventFormEdit, event) + cmd := v.startEventForm(eventFormEdit, event) + // Only close the card if the form was successfully created; if startEventForm + // found no fileable calendars it returns an error notice without setting + // v.eventForm, so we keep the card open rather than leaving the user with nothing. + if v.eventForm != nil { + v.detail = nil + } + return cmd } return v.detail.update(msg) } diff --git a/internal/tui/styles.go b/internal/tui/styles.go index 1a5a783f..085a3a1a 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -191,8 +191,8 @@ func errorView(errMsg string, width int) string { lines := wrapText(errMsg, maxInner) innerWidth := 6 for _, l := range lines { - if len(l) > innerWidth { - innerWidth = len(l) + if w := displayWidth(l); w > innerWidth { + innerWidth = w } } @@ -205,7 +205,7 @@ func errorView(errMsg string, width int) string { var b strings.Builder b.WriteString(padTo(border.Render("╭─ Error "+strings.Repeat("─", innerWidth-6)+"╮")) + "\n") for _, l := range lines { - pad := strings.Repeat(" ", innerWidth-len(l)) + pad := strings.Repeat(" ", innerWidth-displayWidth(l)) b.WriteString(padTo(border.Render("│")+" "+errStyle.Render(l)+pad+" "+border.Render("│")) + "\n") } b.WriteString(padTo(border.Render("╰"+strings.Repeat("─", innerWidth+2)+"╯")) + "\n") From c17636c7f9fd3bd82124a51c1c60d7973cd867e7 Mon Sep 17 00:00:00 2001 From: albertreig Date: Mon, 14 Sep 2026 15:27:50 +0000 Subject: [PATCH 5/6] Fix Copilot review round 4: unknown repeat, port-only host, linear wrapText MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - event_detail.go: show 'Repeats on a schedule' when Recurring is true but the kind is not a known preset, matching the form's own treatment of unknown schedules (event_form_test.go:742-744). - event_detail.go: use parsed.Hostname() instead of parsed.Host so that a port-only authority like 'https://:443' is correctly rejected. - styles.go: compute displayWidth(w) once per word before the hard-wrap loop and update it after each chunk, keeping the inner loop O(n) instead of O(n²). --- internal/tui/event_detail.go | 7 +++++-- internal/tui/event_detail_test.go | 1 + internal/tui/styles.go | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/tui/event_detail.go b/internal/tui/event_detail.go index 72bb4c20..ddafb04a 100644 --- a/internal/tui/event_detail.go +++ b/internal/tui/event_detail.go @@ -109,7 +109,7 @@ func (d *eventDetail) openableLink() (string, bool) { if scheme != "http" && scheme != "https" { return "", false } - if parsed.Host == "" { + if parsed.Hostname() == "" { return "", false } return link, true @@ -130,7 +130,10 @@ func (d *eventDetail) content() string { b.WriteString(whenLine + "\n") } } - if label := repeatFrequencyLabel(d.event.RepeatKind); d.event.Recurring && label != "" { + if label := repeatFrequencyLabel(d.event.RepeatKind); d.event.Recurring { + if label == "" { + label = "on a schedule" // unknown repeat kind — still recurring, just not a named preset + } b.WriteString(styleMuted.Render("Repeats "+label) + "\n") } diff --git a/internal/tui/event_detail_test.go b/internal/tui/event_detail_test.go index 7c792e76..fd17486b 100644 --- a/internal/tui/event_detail_test.go +++ b/internal/tui/event_detail_test.go @@ -18,6 +18,7 @@ func TestEventDetailOpensOnlyHostedWebLinks(t *testing.T) { {name: "HTTPS link (uppercase scheme)", link: "HTTPS://meet.example.com/roadmap", want: true}, {name: "hostless HTTPS link", link: "https:roadmap", want: false}, {name: "empty HTTPS host", link: "https://", want: false}, + {name: "port only host (https://:443)", link: "https://:443/path", want: false}, {name: "file link", link: "file:///etc/passwd", want: false}, {name: "application scheme", link: "zoomus://zoom.us/join/123", want: false}, {name: "empty link", link: "", want: false}, diff --git a/internal/tui/styles.go b/internal/tui/styles.go index 085a3a1a..3eb4edfd 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -233,7 +233,9 @@ func wrapText(s string, maxWidth int) []string { for _, w := range words { // Hard-wrap any word whose display width alone exceeds maxWidth, advancing by // whole grapheme clusters so we never split inside a rune or emoji sequence. - for displayWidth(w) > maxWidth { + // Compute the word width once; iterate over chunks to keep this linear. + wWidth := displayWidth(w) + for wWidth > maxWidth { if line != "" { lines = append(lines, line) line = "" @@ -245,11 +247,11 @@ func wrapText(s string, maxWidth int) []string { } lines = append(lines, chunk) w = w[len(chunk):] + wWidth = displayWidth(w) } if w == "" { continue } - wWidth := displayWidth(w) if line == "" { line = w lineWidth = wWidth From 7f63f9d793a6030dd5eaa43369df168f07496270 Mon Sep 17 00:00:00 2001 From: albertreig Date: Mon, 14 Sep 2026 15:42:50 +0000 Subject: [PATCH 6/6] Fix Copilot review round 5: stacked label layout, oversized grapheme advance, linear wrapText - event_detail.go: switch to stacked label layout (label truncated to contentWidth, value indented by 2) when contentWidth <= labelWidth, so no row overflows the modal on very narrow terminals. - styles.go: when the leading grapheme of a word is wider than maxWidth, emit it as-is and advance past it (instead of breaking) so the suffix is still wrapped. - styles.go: subtract the emitted chunk width from wWidth instead of scanning the remaining suffix with displayWidth, keeping the hard-wrap loop strictly linear. --- internal/tui/event_detail.go | 27 ++++++++++++++++++------- internal/tui/event_detail_test.go | 33 +++++++++++++++++++++++++++++++ internal/tui/styles.go | 13 +++++++++--- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/internal/tui/event_detail.go b/internal/tui/event_detail.go index ddafb04a..27c4f6a4 100644 --- a/internal/tui/event_detail.go +++ b/internal/tui/event_detail.go @@ -144,7 +144,11 @@ func (d *eventDetail) content() string { {"Guests", d.guests()}, } // The label column is 8 chars + 2 spaces of padding; what remains is for the value. - labelWidth := 10 + // On very narrow terminals the label column alone would exceed contentWidth, so we + // switch to a stacked layout (label on one line, value indented on the next) to keep + // every row within the modal's content width. + const labelWidth = 10 // 8-char label + 2-space gap + stacked := contentWidth <= labelWidth valueWidth := max(contentWidth-labelWidth, 1) wrote := false for _, row := range rows { @@ -155,12 +159,21 @@ func (d *eventDetail) content() string { b.WriteString("\n") wrote = true } - // Wrap long values so no row makes the modal wider than the terminal. - for i, valueLine := range wrapText(row[1], valueWidth) { - if i == 0 { - fmt.Fprintf(&b, "%s %s\n", d.styles.entryFrom.Render(fmt.Sprintf("%-8s", row[0])), valueLine) - } else { - fmt.Fprintf(&b, "%s %s\n", strings.Repeat(" ", 8), valueLine) + if stacked { + // Stacked layout: label on its own line (truncated to contentWidth), value indented by 2. + label := fitGraphemes(row[0], contentWidth) + b.WriteString(d.styles.entryFrom.Render(label) + "\n") + for _, valueLine := range wrapText(row[1], max(contentWidth-2, 1)) { + fmt.Fprintf(&b, " %s\n", valueLine) + } + } else { + // Side-by-side layout: fixed 8-char label, value wrapped to remaining width. + for i, valueLine := range wrapText(row[1], valueWidth) { + if i == 0 { + fmt.Fprintf(&b, "%s %s\n", d.styles.entryFrom.Render(fmt.Sprintf("%-8s", row[0])), valueLine) + } else { + fmt.Fprintf(&b, "%s %s\n", strings.Repeat(" ", 8), valueLine) + } } } } diff --git a/internal/tui/event_detail_test.go b/internal/tui/event_detail_test.go index fd17486b..82ed01e8 100644 --- a/internal/tui/event_detail_test.go +++ b/internal/tui/event_detail_test.go @@ -146,6 +146,19 @@ func TestWrapTextSplitsAtGraphemeBoundaries(t *testing.T) { } } +// When the leading grapheme of a word is wider than maxWidth, wrapText must still advance +// past it and continue wrapping the suffix rather than looping forever or dropping it. +func TestWrapTextAdvancesPastOversizedLeadingGrapheme(t *testing.T) { + // A 3-cell wide emoji followed by ASCII; maxWidth=2 means the emoji cannot fit. + // wrapText should emit the emoji alone and then wrap the rest normally. + s := "🎉abc" + lines := wrapText(s, 2) + rejoined := strings.Join(lines, "") + if rejoined != s { + t.Errorf("wrapText lost characters: got %q, want %q", rejoined, s) + } +} + // On a narrow terminal the when() line must not exceed the modal content width. func TestEventCardWhenLineWrapsOnNarrowTerminal(t *testing.T) { d := &eventDetail{ @@ -162,3 +175,23 @@ func TestEventCardWhenLineWrapsOnNarrowTerminal(t *testing.T) { } } } + +// On a very narrow terminal (contentWidth <= labelWidth) the card must use stacked layout. +func TestEventCardUsesStackedLayoutOnVeryNarrowTerminal(t *testing.T) { + d := &eventDetail{ + event: Recording{ + StartsAt: atLocal("2026-08-20T14:00:00"), EndsAt: atLocal("2026-08-20T15:00:00"), + Location: "Sala 2", + }, + styles: testVC().styles, + width: 10, // very narrow — contentWidth will be <= labelWidth + height: 20, + } + content := d.content() + for _, line := range strings.Split(content, "\n") { + stripped := ansi.Strip(line) + if displayWidth(stripped) > modalContentWidth(10) { + t.Errorf("stacked content line exceeds modal content width on very narrow terminal: %q (width=%d)", stripped, displayWidth(stripped)) + } + } +} diff --git a/internal/tui/styles.go b/internal/tui/styles.go index 3eb4edfd..4ef49c7d 100644 --- a/internal/tui/styles.go +++ b/internal/tui/styles.go @@ -233,7 +233,7 @@ func wrapText(s string, maxWidth int) []string { for _, w := range words { // Hard-wrap any word whose display width alone exceeds maxWidth, advancing by // whole grapheme clusters so we never split inside a rune or emoji sequence. - // Compute the word width once; iterate over chunks to keep this linear. + // Track the remaining width by subtraction to keep the inner loop linear. wWidth := displayWidth(w) for wWidth > maxWidth { if line != "" { @@ -243,11 +243,18 @@ func wrapText(s string, maxWidth int) []string { } chunk := fitGraphemes(w, maxWidth) if chunk == "" { - break // single cluster wider than maxWidth — emit it whole to avoid infinite loop + // The leading grapheme is wider than maxWidth and cannot be split + // further; emit it as-is and advance past it so the loop terminates. + cluster, clusterWidth := firstCluster(w) + lines = append(lines, cluster) + w = w[len(cluster):] + wWidth -= clusterWidth + continue } + chunkWidth := displayWidth(chunk) lines = append(lines, chunk) w = w[len(chunk):] - wWidth = displayWidth(w) + wWidth -= chunkWidth } if w == "" { continue