Skip to content

Use first commit title for multi-commit PRs and fix auto-focus title field#36606

Merged
silverwind merged 28 commits into
go-gitea:mainfrom
silverwind:openpr
Feb 17, 2026
Merged

Use first commit title for multi-commit PRs and fix auto-focus title field#36606
silverwind merged 28 commits into
go-gitea:mainfrom
silverwind:openpr

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Feb 12, 2026

Fixes: #34865

  1. When opening a PR from a branch with multiple commits, use the first (oldest) commit's title as the default title instead of the branch name
  2. Add autofocus to the PR title input field for convenience

When opening a PR from a branch with multiple commits, use the first
(oldest) commit's message as the default title instead of the branch
name. Also add autofocus to the title input so it receives focus when
the PR form is toggled visible.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 12, 2026
@silverwind silverwind requested a review from Copilot February 12, 2026 20:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the compare/PR-creation flow so multi-commit PRs default their title from the oldest commit (instead of the branch name), and improves UX by auto-focusing the title input when the PR form is shown.

Changes:

  • Set default PR title for multi-commit compares to the oldest commit’s summary.
  • Add autofocus to the issue/PR title field in the shared new issue/PR form template.
  • Add an integration test covering multi-commit PR title prefilling behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
routers/web/repo/compare.go Changes default PR title derivation to use the oldest commit summary when multiple commits exist.
templates/repo/issue/new_form.tmpl Adds autofocus to the title <input> used by both new issue and new PR forms.
tests/integration/compare_test.go Adds an integration test intended to validate multi-commit PR title prefilling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/integration/compare_test.go Outdated
Comment thread tests/integration/compare_test.go Outdated
… test

- Set a known commit_summary for the first commit instead of relying on
  implicit default from testEditFileToNewBranch
- Assert that PR title equals the expected oldest commit title instead of
  only checking what it is not

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 12, 2026

Regarding autofocus, I see there is a data-global-init="initInputAutoFocusEnd" attribute which in theory should trigger .focus(), but it does nothing and the element never gets focused. Only HTML autofocus works. Investigating...

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 12, 2026

Analysis by Claude:

Why data-global-init="initInputAutoFocusEnd" doesn't work here

registerGlobalInitFunc handlers are triggered via a MutationObserver that watches for DOM node additions (childList: true). On page load, the observer scans all existing [data-global-init] elements and calls their handlers immediately, then marks them _giteaGlobalInited = true to prevent re-initialization.

On the PR compare page, the form is already in the DOM on page load, just hidden with tw-hidden:

<div class="pullrequest-form {{if not .ExpandNewPrForm}}tw-hidden{{end}}">
  {{template "repo/issue/new_form" .}}
</div>

So initInputAutoFocusEnd fires immediately during page init — calling el.focus() on a hidden element, which is a no-op. The element is then marked as already initialized, so it never runs again.

When the user clicks "New Pull Request" to toggle the form visible, it's just a CSS class change (removing tw-hidden), not a DOM addition, so the MutationObserver doesn't fire. The toggle handler in web_src/js/features/common-button.ts does have a fallback that focuses [autofocus] elements — which is why autofocus is the correct attribute to use here instead of data-global-init.

The data-global-init handler fires on page load when the element is
still hidden (tw-hidden), making the focus() call a no-op. The toggle
handler in common-button.ts already focuses [autofocus] elements when
the panel becomes visible, so use that instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 12, 2026

As per above, I've removed the broken data-global-init="initInputAutoFocusEnd". Native autofocus works fine in Firefox and Chrome when opening to the Compare page and clicking the button. I assume this may be a special browser behaviour regarding hidden elements with autofocus that may have changed at some point in time.

lunny
lunny previously approved these changes Feb 12, 2026
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 12, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

As per above, I've removed the broken data-global-init="initInputAutoFocusEnd". Native autofocus works fine in Firefox and Chrome when opening to the Compare page and clicking the button. I assume this may be a special browser behaviour regarding hidden elements with autofocus that may have changed at some point in time.

No, browsers are not that smart. I did it

image

@wxiaoguang
Copy link
Copy Markdown
Contributor

As per above, I've removed the broken data-global-init="initInputAutoFocusEnd".

It isn't broken. It is designed to move the cursor to the end of the title.

When the title in the issue has a value, set the text cursor at the end of the text. (#30090)

Not only one user asks for this.

@wxiaoguang wxiaoguang marked this pull request as draft February 12, 2026 23:52
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 13, 2026

As per above, I've removed the broken data-global-init="initInputAutoFocusEnd".

It isn't broken. It is designed to move the cursor to the end of the title.

When the title in the issue has a value, set the text cursor at the end of the text. (#30090)

Not only one user asks for this.

Try on main branch, this JS autofocus is not working at all. Cursor is already at the end on this branch.

BTW I think the best autofocus variant would be with selection of the whole <input> so user can immediately type to write a custom title.

@wxiaoguang
Copy link
Copy Markdown
Contributor

As per above, I've removed the broken data-global-init="initInputAutoFocusEnd".

It isn't broken. It is designed to move the cursor to the end of the title.
When the title in the issue has a value, set the text cursor at the end of the text. (#30090)
Not only one user asks for this.

Try on main branch, this JS autofocus is not working at all. Cursor is already at the end on this branch.

Ar you sure?

image image
image image

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 13, 2026

regarless of the current issue, I question that "autofocus on end of input" is good behaviour. Is the most common use case really to append to the title? I think a much more common use case would be to replace the title, so a full selection autofous may make more sense imho.

Personally i don't really care much about this so I can keep the "end" behaviour. in 99% of cases I keep the original PR title I had written earlier.

@wxiaoguang
Copy link
Copy Markdown
Contributor

When the title in the issue has a value, set the text cursor at the end of the text. (#30090)

I question that "autofocus on end of input" is good behaviour. Is the most common use case really to append to the title? I think a much more common use case would be to replace the title

Have you read #30090 ? 30090 is the real use case. And not only one user asked for this behavior, for example: @tyroneyeh #36322 ef649c8

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 13, 2026

I think it's weak use case, but I wont chase that topic futher, so lets keep it focusing at the end of the text.

Restore initInputAutoFocusEnd using an IntersectionObserver so it works
both when the input is immediately visible (new issue page) and when it
starts hidden and is later toggled visible (PR form on compare page).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Feb 14, 2026

Autofocus is back with initInputAutoFocusEnd and is now using a IntersectionObserver. This is needed because on the PR compare page, the <input> is hidden until Compare is clicked so el.focus() was a noop (hidden elements can't receive focus). Now it triggers as soon as the element becomes visible.

The behaviour of "last element" is also restored, but is not doing anything useful because issue/pr title can only be once on the page.

Comment thread web_src/js/features/common-page.ts Outdated
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread web_src/js/features/common-page.ts Outdated
@wxiaoguang

This comment was marked as resolved.

Use registerGlobalInitFunc instead of registerGlobalSelectorFunc for
better performance, and rely on native autofocus + show-panel handler
instead of a separate data-autofocus-on-show-panel mechanism.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

all fixed now and tested.

@silverwind
Copy link
Copy Markdown
Member Author

If you want to do more edits, do them now.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Feb 15, 2026

By your approach, the cursor flickers.

Because by using "autofocus", the element is focused in first time. But the "move to end" happens some time later after DOM ready.


It's up to you

@wxiaoguang
Copy link
Copy Markdown
Contributor

Native autofocus attributes has benefits, it works even when the browser has JS disabled. We should extend native autofocus, not replace it.

The new changes just cause flickers. I insist on that my code is already good enough. If you don't like the "init name", you can change it to others you like. But the approach is almost the perfect one.

I don't see why you still use "JS disabled" as a reason, we all know that Gitea won't work without JS nowadays.

@silverwind
Copy link
Copy Markdown
Member Author

Reverted to JS focus again, it can not be solved cleanly with native autofocus.

@silverwind
Copy link
Copy Markdown
Member Author

Tested the latest changes, working as expected, so lgtm.

@lunny, @lafriks please re-review.

@silverwind silverwind added the type/enhancement An improvement of existing functionality label Feb 17, 2026
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 17, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 17, 2026
@silverwind silverwind merged commit 1b874d1 into go-gitea:main Feb 17, 2026
24 checks passed
@GiteaBot GiteaBot added this to the 1.26.0 milestone Feb 17, 2026
@silverwind silverwind deleted the openpr branch February 17, 2026 08:06
@go-gitea go-gitea locked as resolved and limited conversation to collaborators May 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Idea: Don't pre-fill pull request title with branch name

6 participants