Refreshed Pull Request Commits Page Feedback

[willsmythe]

The pull request commits page has been refreshed to improve performance, improve consistency with other pages, and to make the experience more accessible!

To minimize disruptions, the capabilities of the classic commits page have been maintained, with a few exceptions: you can now use arrow keys to navigate the list of commits (instead of j and k) and focus indicators have been improved for better visual distinction.

Note: The roll-out of this refreshed page will begin on September 4, 2024.

Opt out

To switch back to the classic commits page, disable the "New Pull Request Commits Experience" feature preview (learn more).

Feedback

Have a question? Have feedback? Tell us below!

[JonathanRefera]

Worked well and fast. No bugs found. Good.

[hanzlahabib]

just minor UI

[Cheesidibbl]

agreed

[0657721942]

repo-sync

[Poopy1976bmf]

One of the best android rooting apps

[nathanielop]

On pull requests with several commits- scrolling up too fast causes the header to jump before/after the sticky one is unrendered

[willsmythe]

Thanks for the feedback. Which browser are you seeing this on? And if you're able to capture a video of the problem, can you post it here? If not, no worries, we'll investigate regardless.

[x0000ff]

I have the same behavior.

• macOS 14.6.1 (23G93)
• Chrome Version 117.0.5938.132 (Official Build) (arm64)

Another things I noticed is a red background that are not clipped:

[Kimihoc]

thanks

[maciejs-bp]

The difference in UX is quite big - at least on Chrome/Linux arrow keys scroll the page by default. Even after focusing the commit list with Tab I can only navigate a single commit group and nothing happens after selecting a commit with Return. On the old one I could navigate immediately with j/k (which feels natural to Vim users) and pressing Return opened the commit.

[willsmythe]

Thanks for the feedback.

• We plan on fixing the issue where you can't navigate the full list of commits with the down/up arrow keys
• We also plan on fixing the issue with Return/Enter not navigating to that commit
• We intentionally removed j and k support for accessibility reasons, but we're exploring ways to support them (but not impact accessibility). Stay tuned!

[Manchmal1]

Seher gute interessante Arbeit,
Danke

[bfdifan1]

mhm

[LasseRafn]

• PR Author now has underline under the name, which is inconsistent with other PR UIs
  

• Diff no longer has decimal separators
  

• Scrolling fast, as reported by others, causes the page to jump up and down

[inkblotty]

@LasseRafn - Thank you for the report 🙏 On the first issue, do you have the Link Underline setting turned off? Just for additional context here.

[baptiste-n42]

I noticed that the deletion icons (red cubes) are striped on the commit page but not on the rest of the tabs. Maybe it's normal.

[inkblotty]

Yes @baptiste-n42 - That is an intentional change to allow for better differentiation for people with color-blind disabilities.

[nwnk]

[Araxeus]

This is unrelated to the refresh, it was like that before

[jesseduffield]

This may not be the right place to ask, but I'd like to have the option to view a page which shows file changes commit-by-commit, so it starts at the first commit with its description and file changes, and then you scroll down and eventually you reach the second commit with its file changes, etc. Currently I have to choose between:

1. Just looking at the final diff
2. Opening a new browser tab for each commit and then going through each tab

[thislooksfun]

When viewing a single commit on a PR there are prev/next buttons on the top-right of the commit message box. Additionally pressing n will go to the next commit and p will go to the previous one. It's not an infinite scroll, but it's still (imo) better than using multiple tabs, and I like the organization of having one commit per page so I don't get confused when scrolling back up and accidentally going too far.

[cipacera]

Do you prefer to view file changes commit-by-commit or just look at the final diff in pull requests?

[Araxeus]

Not a big fan of the new buttons, its not very clear at a glance that those are buttons unlike the old design

[Billonwill]

Definitely its irritative design

[jez]

There's a counterintuitive result in interface design where the ease with which a button can be clicked is proportional to the size of the button in the UI, not just the size of the click target.

In the new design, since there is no background around the button by default, it gives the impression to users that they must be precise in clicking only the text, so they'll slow down their mouse cursor as they get closer to the commit text.

If the commit button were to be larger, like it was in the old design, users will understand the click target to be larger and know that they don't have to be so precise in clicking the button, making it easier to click.

Posting because I've noticed myself doing this every time I go to click the new commit buttons, even though in my head I know that the clickable area is the same size as it always was. Would love if we could get a better visual indicator that these are buttons, not just links.

[patrron]

Definitely its irritative design

[Sebastian-Brud-Tectonic]

Broken branch names. It keeps saying release%2FXYZ is not a proper branch name. Broken feature

[aelnosu]

Hi, The verify tag is not being shown consistently. If the commit is signed with S/MIME the verified tag is different between the PR page and that individual patch page.

[aelnosu]

Personally, I would prefer the verified tag on the individual patch page, but I hope there is more space between the profile picture and the title.

[okarny]

I wish there was a view in which you could see when you submitted a review in the series of commits. So I know after I requested for changes then the Engineer makes commits following that I can look at those more closely.

[ShineZaw123]

good

[ghostinhershell]

Hey @ShineZaw123, thanks for commenting! Any particular feedback you would like to share?

[aaronmarkey]

Could we have some way to see the actual date and time commits were made? "Last week" is extraordinarily unhelpful.

[willsmythe]

Across GitHub (and on this page), you should be able to hover your mouse over "last week" or "3 days ago" or whatever and see the exact date and time in a tooltip, for example:

[hsuabina]

I think I found a bug. I have a PR opened, where I'm not seeing the last commit and I can't merge:

The PR view (note it ends in #35c6848e):

If I browse the branch commits, the last one is there (#219f454):

If I try to merge the PR I get this error:

In git (not github) the branch looks OK in the remote.

[hsuabina]

So far so good!

[nexdrew]

My team is seeing this today in one of our private repos. 3 commits on the branch but only 2 show in the PR.

[nexdrew]

Closing the PR and reopening it resolved the problem.

[inkblotty]

Thanks for reporting @nexdrew. Maybe this is an occasional edge case we need to hunt down. Would you let us know if you see it again?

[nexdrew]

@inkblotty Will do

[bigsmile74]

Where is the "squash button" ?

Please add an extra column witch check boxes along side the commits.
Then a user can select a number of commits and press squash.

I need this ;) badly.

[adlai]

I think one of the new features might do this. When I scroll until "This branch has no conflicts with the base branch", there is a dropdown arrow next to the Rebase / Merge button; one of the options there is "Squash and Merge".

If you have trouble finding this, I can try searching the official documentation for a better guide.

EDIT: It is at the end of the default PR view, the "Conversation" tab, found to the left of this "Commits" tab when scrolling to the top of the page.

[cormacrelf]

Viewing a commit's extended description is unnecessarily painful. When you click the [...] button, it shows a popover saying "Hide description for 0a1b2c". That popover covers the text of the description, and it is also stuck until you click somewhere else on the page. So you can't read the description without clicking twice.

This is especially bad if you are reading more than one commit description. You must click 2N times. Not only would I like to click a maximum of N times, I would also like an option to show all descriptions, all the time. If I'm on the commits page, it is so I can read the descriptions and see what someone has done.

In all, I suggest:

• Move the popover to the right hand side of the [...]
• Only show the popover when hovered, so it goes away as soon as you move the mouse
• Or just get rid of the popover. Who needs this? If you've opened it, you know what the button does already.
• Add an option somewhere to show all descriptions by default

[HBelusca]

There seems to exist a drawing artifact when showing up the "Review Changes" / "Finish the review" dialog: see inside the red-circle, where the text "of review" shown behind the "Comment" radio-button:

[kuroshrk-arista]

I wonder if PR review comments should at least list (behind a button or otherwise) underneath commits on the commits page;
a natural way to organize the sequential reviews and discussion, given subsequent commits should show another morphology of the code?

pain point

• it can prove difficult to find and determine the sequence of code comments, over multiple reviews especially.

Information architecture caveat
of course one will have to then retain comments on removed commits, upon reset and force-push...; simply hiding behind a [subtle] horizontal rule could work there (global option to show or hide review comments for formerly exigent commits, on this commit page/section)

[p-soma]

I force pushed my branch to remove the most recent commit, but that commit still shows up in the commit list on the PR. Viewing the branch on Github, I can see that the branch is updated to no longer have that commit. On the main PR page, there is a "processing updates" spinner. Has been like this since yesterday.

[zarghol]

Nice page. One suggestion I have is to see the additions / deletions graph for each commit, in order to check the "size" of the commits

[Surphr]

มีการรีเฟรชหน้าการยืนยันการดึงคำขอเพื่อปรับปรุงประสิทธิภาพ ปรับปรุงความสอดคล้องกับหน้าอื่นๆ และเพื่อให้ประสบการณ์เข้าถึงได้ง่ายยิ่งขึ้น!

เพื่อลดการหยุดชะงักให้เหลือน้อยที่สุด ความสามารถของหน้าคอมมิทแบบคลาสสิกจึงได้รับการรักษาไว้ โดยมีข้อยกเว้นบางประการ ได้แก่ ตอนนี้คุณสามารถใช้ปุ่มลูกศรเพื่อนำทางไปยังรายการคอมมิท (แทนที่จะเป็นjและk) และตัวระบุโฟกัสได้รับการปรับปรุงเพื่อการแยกแยะภาพที่ดีขึ้น

หมายเหตุ: การเปิดตัวหน้าเว็บที่ปรับปรุงใหม่นี้จะเริ่มในวันที่ 4 กันยายน 2024

ยกเลิกเลือก

หากต้องการสลับกลับไปยังหน้าการยืนยันแบบคลาสสิก ให้ปิดใช้งานคุณลักษณะการดูตัวอย่าง "ประสบการณ์การยืนยันแบบ Pull Request ใหม่" ( เรียนรู้เพิ่มเติม )

ข้อเสนอแนะ

มีคำถามหรือไม่? มีข้อเสนอแนะหรือไม่? บอกเราด้านล่างได้เลย!

ผมไม่บ้าไม่ได้หลอน....

[abelykh0]

Please sort commits descending by date

What I see:

[pcadungog]

It would be nice to have a sorting order option for commits which the latest is on top.

Similar to this.

[FelixFurtmayr]

yes, this must be a mistake. you always want to see what is latest.

[surfdude29]

I don't know how best to describe this, but when I scroll down to the bottom of this page using the in-app browser on my iPhone running iOS 17.7.2 things get seriously weird 😬

RPReplay_Final1732556817.mov

[juri]

Yeah, I'm seeing this with desktop Safari too.

[adlai]

It would be awesome if there were an expander, similar to the ellipsis shown for multiline commit messages, that displayed the output git show --stat for that commit; this could be a good compromise towards resembling git log --stat, which could be too verbose.

[adlai]

I've found a porcelain command that more closely approximates the view I had in mind when writing the original toplevel comment: git log --stat --pretty=oneline --

In case you're copying this for alias and reuse, the two hyphens are followed by a space so the intended effect is letting you filter by files, while still displaying which other files were affected by each commit; I was pleasantly surprised to discover that the --stat and =oneline are not mutually-exclusive!

I'll close with a minimal sample of the command's output, for the folks who don't currently have access to the git porcelain tools while reading on the website or from email notifications:

% git log --stat --pretty=oneline -- hotfails/
6c140b1d4092b4608764f2f0f3b8af518928a9a5 Delete hotfails/scripts.log
 hotfails/scripts.log | 11 -----------
 1 file changed, 11 deletions(-)
ce6de801b44f0c95a326d97a5abecb558c803f8c fixing bugs
 hotfails/old_fail.txt | 5 +++++
 1 file changed, 5 insertions(+)
762b5a9749ad8d05cfcbba891639a3aa5791e40c THIS COMMIT IS NOT 1337
 hotfails/next.dat | 4 ++++
 1 file changed, 4 insertions(+)

[jfrost-mo]

The list of commits seems to get incorrectly cached, and not updated when I navigate to it after rebasing. Refreshing doesn't fix it, but hard refreshing (Ctrl-F5) does fix it.

This might be related to me being behind a proxy that breaks most websockets.

[0657721942]

h

[typed-sigterm]

You're all set — the branch has been merged.

In my opinion, this sentence should not always appear. When I'm not an author/committer/reviewer of this PR, it can be hidden 🤔

[adlai]

As a prospective reviewer of any PR, you should probably be aware of whether it has been merged; vanilla GitHub isn't for double-blind competence trials.