Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions internal/tui/components/notificationssection/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,22 @@ Similarly, when viewing an Issue notification, Issue-specific keybindings are av

The `?` help display dynamically updates to show the applicable keybindings based on what type of notification content is being viewed.

#### Confirmation Prompts for Destructive Actions

When viewing a PR or Issue notification, destructive actions (close, reopen, merge, etc.) require confirmation before execution. This uses a footer-based confirmation mechanism separate from the section-level confirmation used in PR/Issue views:

1. User presses action key (e.g., `x` for close)
2. Footer displays: "Are you sure you want to close PR #123? (y/N)"
3. User presses `y`, `Y`, or `Enter` to confirm, any other key cancels
4. Action executes via the `tasks` package (same as PR/Issue views)

This design is necessary because:
- The notification section doesn't understand PR/Issue-specific actions
- PR/Issue data is stored in `notificationView`, not in the section
- Actions operate on the notification's subject PR/Issue, not the notification itself

The `pendingNotificationAction` field in the Model tracks the pending action (e.g., "pr_close", "issue_reopen") until confirmed or cancelled.

## Configuration

### Search Section
Expand Down
6 changes: 6 additions & 0 deletions internal/tui/components/notificationview/notificationview.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ func (m *Model) SetWidth(width int) {
m.width = width
}

func (m *Model) ResetSubject() {
m.subjectPR = nil
m.subjectIssue = nil
m.subjectId = ""
}

func (m *Model) SetSubjectPR(pr *prrow.Data, notificationId string) {
m.subjectPR = pr
m.subjectIssue = nil
Expand Down
2 changes: 1 addition & 1 deletion internal/tui/components/prrow/prrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (pr *PullRequest) renderExtendedTitle(isSelected bool) string {
baseStyle = baseStyle.Foreground(pr.Ctx.Theme.SecondaryText).Background(pr.Ctx.Theme.SelectedBackground)
}

author := baseStyle.Bold(true).Foreground(pr.Ctx.Theme.FaintText).Render(fmt.Sprintf("@%s",
author := baseStyle.Bold(true).Render(fmt.Sprintf("@%s",
pr.Data.Primary.GetAuthor(pr.Ctx.Theme, pr.ShowAuthorIcon)))
top := lipgloss.JoinHorizontal(lipgloss.Top, pr.Data.Primary.Repository.NameWithOwner,
fmt.Sprintf(" #%d by %s", pr.Data.Primary.Number, author))
Expand Down
227 changes: 200 additions & 27 deletions internal/tui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package tui
import (
"fmt"
"os"
"os/exec"
"reflect"
"runtime/debug"
"sort"
Expand Down Expand Up @@ -37,6 +38,7 @@ import (
"github.com/dlvhdr/gh-dash/v4/internal/tui/components/section"
"github.com/dlvhdr/gh-dash/v4/internal/tui/components/sidebar"
"github.com/dlvhdr/gh-dash/v4/internal/tui/components/tabs"
"github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks"
"github.com/dlvhdr/gh-dash/v4/internal/tui/constants"
"github.com/dlvhdr/gh-dash/v4/internal/tui/context"
"github.com/dlvhdr/gh-dash/v4/internal/tui/keys"
Expand All @@ -60,6 +62,12 @@ type Model struct {
ctx *context.ProgramContext
taskSpinner spinner.Model
tasks map[string]context.Task

// Pending confirmation action for notification PR/Issue (close, merge, etc.)
// This is separate from section confirmation because PR/Issue actions in
// notification view need to be executed on the notification's subject PR/Issue,
// not on the notification section itself.
pendingNotificationAction string // "pr_close", "pr_merge", "issue_close", etc.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this live in the notificationsview? well if we move some logic there it
makes sense

}

func NewModel(location config.Location) Model {
Expand Down Expand Up @@ -204,6 +212,17 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, nil
}

// Handle notification PR/Issue action confirmation
if m.pendingNotificationAction != "" {
if msg.String() == "y" || msg.String() == "Y" || msg.Type == tea.KeyEnter {
cmd = m.executeNotificationAction()
}
// Any other key cancels the confirmation
m.pendingNotificationAction = ""
m.footer.SetLeftSection("")
return m, cmd
}
Comment on lines +216 to +224
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering, what's preventing the notificationsview from handling
confirmation the same as the other views? e.g. in https://github.com/dlvhdr/gh-dash/blob/main/internal/tui/components/prview/prview.go?plain=1#L100


switch {
case m.isUserDefinedKeybinding(msg):
cmd = m.executeKeybinding(msg.String())
Expand Down Expand Up @@ -488,9 +507,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {

// PR keybindings when viewing a PR notification
case m.notificationView.GetSubjectPR() != nil:
var prCmd tea.Cmd
m.prView, prCmd = m.prView.Update(msg)

// Check for PR actions first (before updating prView)
if !m.prView.IsTextInputBoxFocused() {
action := prview.MsgToAction(msg)
if action != nil {
Expand All @@ -509,30 +526,37 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {

case prview.PRActionDiff:
if pr := m.notificationView.GetSubjectPR(); pr != nil {
cmd = common.DiffPR(pr.GetNumber(), pr.GetRepoNameWithOwner(), m.ctx.Config.GetFullScreenDiffPagerEnv())
cmd = common.DiffPR(pr.GetNumber(), pr.GetRepoNameWithOwner(),
m.ctx.Config.GetFullScreenDiffPagerEnv())
}
return m, cmd

case prview.PRActionCheckout:
if pr := m.notificationView.GetSubjectPR(); pr != nil {
cmd, _ = notificationssection.CheckoutPR(m.ctx, pr.GetNumber(), pr.GetRepoNameWithOwner())
cmd, _ = notificationssection.CheckoutPR(
m.ctx, pr.GetNumber(), pr.GetRepoNameWithOwner())
}
return m, cmd

case prview.PRActionClose:
return m, m.promptConfirmation(currSection, "close")
cmd = m.promptConfirmationForNotificationPR("close")
return m, cmd

case prview.PRActionReady:
return m, m.promptConfirmation(currSection, "ready")
cmd = m.promptConfirmationForNotificationPR("ready")
return m, cmd

case prview.PRActionReopen:
return m, m.promptConfirmation(currSection, "reopen")
cmd = m.promptConfirmationForNotificationPR("reopen")
return m, cmd

case prview.PRActionMerge:
return m, m.promptConfirmation(currSection, "merge")
cmd = m.promptConfirmationForNotificationPR("merge")
return m, cmd

case prview.PRActionUpdate:
return m, m.promptConfirmation(currSection, "update")
cmd = m.promptConfirmationForNotificationPR("update")
return m, cmd

case prview.PRActionSummaryViewMore:
m.prView.SetSummaryViewMore()
Expand All @@ -542,8 +566,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
}
}

// Always sync and return after updating prView - needed for tab navigation
// which updates carousel state but doesn't return a command
// No action matched - update prView for navigation (tab switching, scrolling)
var prCmd tea.Cmd
m.prView, prCmd = m.prView.Update(msg)
m.syncSidebar()
return m, prCmd
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to use use cmds = append(cmds, prCmd) instead of return m, prCmd
since the scrolling happens in the sidebar component which gets updated here. Right now scrolling in the sidebar with Ctrl+D etc doesn't work.


Expand All @@ -568,37 +593,32 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, m.openSidebarForInput(m.issueSidebar.SetIsCommenting)

case issueview.IssueActionClose:
return m, m.promptConfirmation(currSection, "close")
cmd = m.promptConfirmationForNotificationIssue("close")
return m, cmd

case issueview.IssueActionReopen:
return m, m.promptConfirmation(currSection, "reopen")
cmd = m.promptConfirmationForNotificationIssue("reopen")
return m, cmd
}
}

if issueCmd != nil {
m.syncSidebar()
return m, issueCmd
}
// Sync sidebar and return issueCmd for navigation
m.syncSidebar()
return m, issueCmd
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


// Notification-specific keybindings
// Notification-specific keybindings (when not viewing PR/Issue content)
case key.Matches(msg, keys.NotificationKeys.View):
// View notification content and mark as read
cmds = append(cmds, m.loadNotificationContent())

case key.Matches(msg, keys.NotificationKeys.MarkAsDone):
// Already handled in the section's Update method
cmd = m.updateSection(currSection.GetId(), currSection.GetType(), msg)
return m, cmd

case key.Matches(msg, keys.NotificationKeys.MarkAllAsDone):
if currSection != nil {
currSection.SetPromptConfirmationAction("done_all")
cmd = currSection.SetIsPromptConfirmationShown(true)
}
cmd = m.promptConfirmation(currSection, "done_all")
return m, cmd

case key.Matches(msg, keys.NotificationKeys.Open):
// Handled in the section's Update method
cmd = m.updateSection(currSection.GetId(), currSection.GetType(), msg)
return m, cmd

Expand Down Expand Up @@ -929,6 +949,7 @@ func (m *Model) onViewedRowChanged() tea.Cmd {
sidebarCmd := m.syncSidebar()
enrichCmd := m.prView.EnrichCurrRow()
m.sidebar.ScrollToTop()
m.notificationView.ResetSubject()
return tea.Batch(sidebarCmd, enrichCmd)
}

Expand Down Expand Up @@ -1315,7 +1336,8 @@ func (m *Model) setCurrentViewSections(newSections []section.Section) {

// Handle notifications view with search section like PRs/Issues
if m.ctx.View == config.NotificationsView {
missingSearchSection := len(newSections) == 0 || (len(newSections) > 0 && newSections[0].GetId() != 0)
missingSearchSection := len(newSections) == 0 ||
(len(newSections) > 0 && newSections[0].GetId() != 0)
s := make([]section.Section, 0)
if missingSearchSection {
// Check if we have an existing search section to preserve
Expand Down Expand Up @@ -1553,3 +1575,154 @@ func (m *Model) doUpdateFooterAtInterval() tea.Cmd {
},
)
}

// promptConfirmationForNotificationPR shows a confirmation prompt for PR actions
// when viewing a PR from a notification. This is separate from section-based
// confirmation because the notification section doesn't know about PR actions.
func (m *Model) promptConfirmationForNotificationPR(action string) tea.Cmd {
pr := m.notificationView.GetSubjectPR()
if pr == nil {
return nil
}

m.pendingNotificationAction = "pr_" + action

// Show confirmation in footer
actionDisplay := action
if action == "ready" {
actionDisplay = "mark as ready"
}
prompt := fmt.Sprintf("Are you sure you want to %s PR #%d? (y/N)", actionDisplay, pr.GetNumber())
m.footer.SetLeftSection(m.ctx.Styles.ListViewPort.PagerStyle.Render(prompt))

return nil
}

// promptConfirmationForNotificationIssue shows a confirmation prompt for Issue actions
// when viewing an Issue from a notification.
func (m *Model) promptConfirmationForNotificationIssue(action string) tea.Cmd {
issue := m.notificationView.GetSubjectIssue()
if issue == nil {
return nil
}

m.pendingNotificationAction = "issue_" + action

// Show confirmation in footer
prompt := fmt.Sprintf("Are you sure you want to %s Issue #%d? (y/N)", action, issue.Number)
m.footer.SetLeftSection(m.ctx.Styles.ListViewPort.PagerStyle.Render(prompt))

return nil
}

// executeNotificationAction executes the pending PR/Issue action after user confirmation
func (m *Model) executeNotificationAction() tea.Cmd {
action := m.pendingNotificationAction
if action == "" {
return nil
}

// Create a section identifier for the notification section
currSection := m.getCurrSection()
sid := tasks.SectionIdentifier{Id: 0, Type: notificationssection.SectionType}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use m.currSectionId

if currSection != nil {
sid.Id = currSection.GetId()
}

switch action {
case "pr_close":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.ClosePR(m.ctx, sid, pr)
}
case "pr_reopen":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.ReopenPR(m.ctx, sid, pr)
}
case "pr_ready":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.PRReady(m.ctx, sid, pr)
}
case "pr_merge":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.MergePR(m.ctx, sid, pr)
}
case "pr_update":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.UpdatePR(m.ctx, sid, pr)
}
case "issue_close":
if issue := m.notificationView.GetSubjectIssue(); issue != nil {
return m.closeIssue(sid, issue)
}
case "issue_reopen":
if issue := m.notificationView.GetSubjectIssue(); issue != nil {
return m.reopenIssue(sid, issue)
}
}
Comment on lines +1632 to +1661
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
switch action {
case "pr_close":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.ClosePR(m.ctx, sid, pr)
}
case "pr_reopen":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.ReopenPR(m.ctx, sid, pr)
}
case "pr_ready":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.PRReady(m.ctx, sid, pr)
}
case "pr_merge":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.MergePR(m.ctx, sid, pr)
}
case "pr_update":
if pr := m.notificationView.GetSubjectPR(); pr != nil {
return tasks.UpdatePR(m.ctx, sid, pr)
}
case "issue_close":
if issue := m.notificationView.GetSubjectIssue(); issue != nil {
return m.closeIssue(sid, issue)
}
case "issue_reopen":
if issue := m.notificationView.GetSubjectIssue(); issue != nil {
return m.reopenIssue(sid, issue)
}
}
pr := m.notificationView.GetSubjectPR()
issue := m.notificationView.GetSubjectIssue();
switch action {
case "pr_close":
if pr != nil {
return tasks.ClosePR(m.ctx, sid, pr)
}
case "pr_reopen":
if pr != nil {
return tasks.ReopenPR(m.ctx, sid, pr)
}
case "pr_ready":
if pr != nil {
return tasks.PRReady(m.ctx, sid, pr)
}
case "pr_merge":
if pr != nil {
return tasks.MergePR(m.ctx, sid, pr)
}
case "pr_update":
if pr != nil {
return tasks.UpdatePR(m.ctx, sid, pr)
}
case "issue_close":
if issue != nil {
return m.closeIssue(sid, issue)
}
case "issue_reopen":
if issue != nil {
return m.reopenIssue(sid, issue)
}
}


return nil
}

// closeIssue executes the close action for an Issue using gh CLI
func (m *Model) closeIssue(sid tasks.SectionIdentifier, issue *data.IssueData) tea.Cmd {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issueNumber := issue.Number
repoName := issue.GetRepoNameWithOwner()
taskId := fmt.Sprintf("issue_close_%d", issueNumber)
task := context.Task{
Id: taskId,
StartText: fmt.Sprintf("Closing Issue #%d", issueNumber),
FinishedText: fmt.Sprintf("Issue #%d has been closed", issueNumber),
State: context.TaskStart,
Error: nil,
}
startCmd := m.ctx.StartTask(task)
return tea.Batch(startCmd, func() tea.Msg {
c := exec.Command(
"gh",
"issue",
"close",
fmt.Sprint(issueNumber),
"-R",
repoName,
)
err := c.Run()
return constants.TaskFinishedMsg{
TaskId: taskId,
SectionId: sid.Id,
SectionType: sid.Type,
Err: err,
}
})
}

// reopenIssue executes the reopen action for an Issue using gh CLI
func (m *Model) reopenIssue(sid tasks.SectionIdentifier, issue *data.IssueData) tea.Cmd {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issueNumber := issue.Number
repoName := issue.GetRepoNameWithOwner()
taskId := fmt.Sprintf("issue_reopen_%d", issueNumber)
task := context.Task{
Id: taskId,
StartText: fmt.Sprintf("Reopening Issue #%d", issueNumber),
FinishedText: fmt.Sprintf("Issue #%d has been reopened", issueNumber),
State: context.TaskStart,
Error: nil,
}
startCmd := m.ctx.StartTask(task)
return tea.Batch(startCmd, func() tea.Msg {
c := exec.Command(
"gh",
"issue",
"reopen",
fmt.Sprint(issueNumber),
"-R",
repoName,
)
err := c.Run()
return constants.TaskFinishedMsg{
TaskId: taskId,
SectionId: sid.Id,
SectionType: sid.Type,
Err: err,
}
})
}
Loading