Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should checkmark on successful pr merge be styled as magenta or green? #9047

Closed
williammartin opened this issue May 4, 2024 · 2 comments
Closed
Labels
accessibility gh-pr relating to the gh pr command

Comments

@williammartin
Copy link
Member

Description

I'm creating this from a discussion on slack, where a user felt friction that that it appeared their merge might have errored or failed in some way because their theme had ANSI magenta as more red/pink than purple:

Image

The choice of magenta was chosen to signal similarity to the GitHub Web UI colours where merged PRs are represented with a purple:

Image

While there are other places like issue list (screenshot below) that use magenta styling to indicate the state of the issue and we probably don't want to change those, using magenta to indicate the success or failure of an operation may be something we want to change to green since the colour choice there has a bit more implication.

Image

@williammartin williammartin added the gh-pr relating to the gh pr command label May 4, 2024
@andyfeller
Copy link
Contributor

Code in question and its history

Digging into the history behind gh pr merge, it apparently was once green / success and was intentionally changed in #2799 as an engineering and design decision.

action := "Merged"
switch payload.method {
case PullRequestMergeMethodRebase:
action = "Rebased and merged"
case PullRequestMergeMethodSquash:
action = "Squashed and merged"
}
return m.infof("%s %s pull request %s#%d (%s)\n", m.cs.SuccessIconWithColor(m.cs.Magenta), action, ghrepo.FullName(m.baseRepo), m.pr.Number, m.pr.Title)

Highly subjective with no clear right answer

I can understand the current state and the suggestion equally as both are equally valid:

  • "Successful actions should be green like the Primer CLI foundations says".

    Screenshot of Primer CLI foundation about color roles in GitHub CLI

  • "Merging PRs should be magenta because the GitHub PR timeline uses magenta for the merged action as well as the state"

    Zoomed in screenshot of cli/cli PR #9035 showing purple background

Supporting terminal accessibility using base 16 colors means this is complicated

The GitHub CLI is working to improve the accessibility of the GitHub CLI including how color is used and ensuring that color is not the only contextual distinction when used. This is relevant to this conversation because it limits CLI applications like gh to a very small palette of colors which terminals choose how to render and users can override:

Screenshot of Mac Terminal profiles preferences screen, showcasing user's ability to customize rendering of ANSI colors from 0 to 15

@williammartin
Copy link
Member Author

After discussing this internally we are content with the decision to use ANSI magenta to represent states relating to merging or merged as documented in our Primer CLI Foundations. For most colour palettes, this colour is similar enough to the Web UI to share meaning, and not to appear as an error.

If our users choose to adjust the ANSI colours in ways that might indicate something else, that's their choice but it's not something we want to address in this case where it is a part of our core design palette.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility gh-pr relating to the gh pr command
Projects
None yet
Development

No branches or pull requests

2 participants