From 409e3ca08caffe21b4e3c6a09915ffc55eea5b49 Mon Sep 17 00:00:00 2001 From: nilvng Date: Fri, 6 Dec 2024 19:16:00 +1100 Subject: [PATCH] issue #2329: simplify the UI of the prompt This commit reverts the previous color changes in the prompt UI. While color highlighting could potentially improve the visual appealing of the prompt using the existing color library (mguz/ansi) with the prompt library (AlecAivazis/survey) caused unintended side effects. It reset the bold text style for the selected option. We decide to that bold text style would have a higher priority than the color text ,for three reasons: 1. To maintain consistency with other prompts in the UI and prioritize accessibility 2. While color can enhance the user experience, according to Primer Design Guidelines, color should not be relied upon to convey essential information. 3. visual indicator of the selected option, especially crucial when dealing with long PR titles or branch names. As a future improvement, we may consider a separate issue or PR to address the color library issue and explore controlled color usage in prompts. This could potentially allow for more nuanced visual differentiation while avoiding unintended style resets. Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com> --- pkg/cmd/pr/checkout/checkout.go | 16 ++++++++-------- pkg/cmd/pr/shared/display.go | 25 ------------------------- 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index adaa6abe6..e60658955 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -102,7 +102,7 @@ func checkoutRun(opts *CheckoutOptions) error { return err } - pr, err := resolvePR(client, baseRepo, opts.Prompter, opts.SelectorArg, opts.Interactive, opts.Finder, opts.IO.ColorScheme()) + pr, err := resolvePR(client, baseRepo, opts.Prompter, opts.SelectorArg, opts.Interactive, opts.Finder) if err != nil { return err } @@ -291,7 +291,7 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm return nil } -func resolvePR(httpClient *http.Client, baseRepo ghrepo.Interface, prompter shared.Prompter, pullRequestSelector string, isInteractive bool, pullRequestFinder shared.PRFinder, cs *iostreams.ColorScheme) (*api.PullRequest, error) { +func resolvePR(httpClient *http.Client, baseRepo ghrepo.Interface, prompter shared.Prompter, pullRequestSelector string, isInteractive bool, pullRequestFinder shared.PRFinder) (*api.PullRequest, error) { // When non-interactive if pullRequestSelector != "" { pr, _, err := pullRequestFinder.Find(shared.FindOptions{ @@ -332,19 +332,19 @@ func resolvePR(httpClient *http.Client, baseRepo ghrepo.Interface, prompter shar return nil, err } - pr, err := promptForPR(prompter, cs, *listResult) + pr, err := promptForPR(prompter, *listResult) return pr, err } -func promptForPR(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs api.PullRequestAndTotalCount) (*api.PullRequest, error) { +func promptForPR(prompter shared.Prompter, jobs api.PullRequestAndTotalCount) (*api.PullRequest, error) { candidates := []string{} for _, pr := range jobs.PullRequests { - candidates = append(candidates, text.Truncate(120, fmt.Sprintf("%s %s [%s]", - shared.PRNumberWithColor(cs, pr), + candidates = append(candidates, fmt.Sprintf("#%d %s [%s]", + pr.Number, text.RemoveExcessiveWhitespace(pr.Title), - cs.Gray(pr.HeadLabel()), - ))) + pr.HeadLabel(), + )) } selected, err := prompter.Select("Select a pull request", "", candidates) diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index ffc4b4dce..7bd306104 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -17,15 +17,6 @@ func StateTitleWithColor(cs *iostreams.ColorScheme, pr api.PullRequest) string { return prStateColorFunc(text.Title(pr.State)) } -// PRNumberWithColor returns a colored string representation of a pull request number -// based on its state (open, closed, merged, or draft). -// It prefixes the number with a hash symbol (#) to indicate it's a pull request. -func PRNumberWithColor(cs *iostreams.ColorScheme, pr api.PullRequest) string { - prStateColorFunc := cs.ColorFromRGB(ColorHexCodeForPRState(pr)) - prNumber := fmt.Sprintf("#%d", pr.Number) - return prStateColorFunc(text.Title(prNumber)) -} - func ColorForPRState(pr api.PullRequest) string { switch pr.State { case "OPEN": @@ -42,22 +33,6 @@ func ColorForPRState(pr api.PullRequest) string { } } -func ColorHexCodeForPRState(pr api.PullRequest) string { - switch pr.State { - case "OPEN": - if pr.IsDraft { - return "808080" - } - return "00FF00" - case "CLOSED": - return "FF0000" - case "MERGED": - return "FFA500" - default: - return "" - } -} - func ColorForIssueState(issue api.Issue) string { switch issue.State { case "OPEN":