link to file from its history #27354

denyskon · 2023-09-29T18:24:19Z

Fixes #3852
Fixes #26707

Add a button on file history which directs you to the file at the selected commit.

Co-authored-by: silverwind me@silverwind.io

silverwind · 2023-09-29T18:52:13Z

Also fixes #26707 which might be duplicate.

silverwind · 2023-09-29T18:53:01Z

Can we label the column "Actions" like I did in #26998?

templates/repo/commits_list.tmpl

Co-authored-by: silverwind <me@silverwind.io>

denyskon · 2023-09-29T19:08:55Z

@silverwind I applied the suggested changes

silverwind · 2023-09-29T19:18:18Z

Can we move the "Copy SHA" button to Operations as well? Same styling and all. Wrap it in a <div class="gt-df gt-gap-3>.

denyskon · 2023-09-29T19:35:19Z

Done

silverwind · 2023-09-29T19:41:45Z

Did some tweaks:

delvh · 2023-09-29T19:38:36Z

templates/repo/commits_list.tmpl

 					<th class="two wide right aligned">{{ctx.Locale.Tr "repo.commits.date"}}</th>
+					<th class="one wide right aligned">{{ctx.Locale.Tr "repo.commit.operations"}}</th>


Sounds a bit weird to waste so much space simply because of the column title (that isn't even particularly helpful).
What about leaving it empty instead?

Suggested change

<th class="one wide right aligned">{{ctx.Locale.Tr "repo.commit.operations"}}</th>

<th class="one wide right aligned"></th>

I was thinking about getting rid of the whole table header. For example, branch list is without header as well.

Is this tiny PR just being blown up to a full commit list redesign? 😆

I think the first action is simply not including this column header.
The rest can be left for other PRs.

I guess it looks alright even without the header here.

I think in the future we should get rid of the HTML table completely. For now - I'm fine with leaving the title empty.

Yes, looks acceptable:

silverwind · 2023-09-29T19:48:01Z

The "View File" name is inaccurate when viewing a directory's history. Maybe we should use the same or similar string that GitHub does? "View at this point in the history", I would omit the the in the string.

denyskon · 2023-09-30T08:59:28Z

@silverwind Where did you push your changes? 😆

Did some tweaks:

delvh · 2023-09-30T09:11:32Z

Could you please update the screenshot in the PR description?

denyskon · 2023-09-30T09:42:02Z

I'd wait for the tweaks by silverwind as they change the appearance a bit

puni9869 · 2023-09-30T11:10:03Z

templates/repo/commits_list.tmpl

@@ -76,6 +76,10 @@
 						{{else}}
 							<td class="text right aligned">{{TimeSince .Author.When ctx.Locale}}</td>
 						{{end}}
+						<td class="text right aligned">
+							<button class="btn interact-bg" data-clipboard-text="{{.ID}}">{{svg "octicon-copy"}}</button>
+							{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}


Suggested change

{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}

{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}

could you shift this to backend {{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}, we don't want regression.

puni9869 · 2023-09-30T11:13:04Z

Date column header can be left aligned.

silverwind · 2023-09-30T11:22:21Z

Ah it seems it did not push, I was getting "remote rejected", will check.

silverwind · 2023-09-30T22:35:26Z

Seems I still can not push (is collaboration enabled on this PR?):

 ! [remote rejected]       denyskon/commit-file-link -> denyskon/commit-file-link (permission denied)

Here is the patch:

diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 8667e77a89..ec37d64fec 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -96,8 +96,9 @@ disabled = Disabled
 locked = Locked

 copy = Copy
 copy_url = Copy URL
+copy_hash = Copy hash
 copy_content = Copy content
 copy_branch = Copy branch name
 copy_success = Copied!
 copy_error = Copy failed
diff --git a/templates/repo/commits_list.tmpl b/templates/repo/commits_list.tmpl
index c5003af4ae..db658de3e7 100644
--- a/templates/repo/commits_list.tmpl
+++ b/templates/repo/commits_list.tmpl
@@ -75,11 +75,13 @@
 							<td class="text right aligned">{{TimeSince .Committer.When ctx.Locale}}</td>
 						{{else}}
 							<td class="text right aligned">{{TimeSince .Author.When ctx.Locale}}</td>
 						{{end}}
-						<td class="text right aligned">
-							<button class="btn interact-bg" data-clipboard-text="{{.ID}}">{{svg "octicon-copy"}}</button>
-							{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}
+						<td class="gt-pt-0 gt-pb-0">
+							<div class="gt-df gt-je">
+								<button class="btn interact-bg gt-p-3" data-tooltip-content="{{ctx.Locale.Tr "copy_hash"}}" data-clipboard-text="{{.ID}}">{{svg "octicon-copy"}}</button>
+								{{if $.FileName}}<a class="btn interact-bg gt-p-3" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}
+							</div>
 						</td>
 					</tr>
 				{{end}}
 			</tbody>

wxiaoguang · 2023-10-01T02:16:41Z

I like <td class="text right aligned"> more than the massive gt helpers.

…to commit-file-link

denyskon · 2023-10-01T11:43:13Z

@delvh @silverwind I pushed the changes and updated the screenshot.

silverwind · 2023-10-02T09:26:18Z

I like <td class="text right aligned"> more than the massive gt helpers.

Need the flexbox to vertically center and for the gap. Text-based layout is not suitable for these tasks.

silverwind · 2023-10-02T14:11:39Z

options/locale/locale_en-US.ini

@@ -1283,6 +1284,7 @@ commits.signed_by_untrusted_user = Signed by untrusted user
 commits.signed_by_untrusted_user_unmatched = Signed by untrusted user who does not match committer
 commits.gpg_key_id = GPG Key ID
 commits.ssh_key_fingerprint = SSH Key Fingerprint
+commits.view_path=View at this point in history


If there is a ini linter, I would make it lint for spaces around = 😆

wxiaoguang · 2023-10-02T14:34:06Z

templates/repo/commits_list.tmpl

+						<td class="gt-pt-0 gt-pb-0">
+							<div class="gt-df gt-je">
+								<button class="btn interact-bg gt-p-3" data-tooltip-content="{{ctx.Locale.Tr "copy_hash"}}" data-clipboard-text="{{.ID}}">{{svg "octicon-copy"}}</button>
+								{{if $.FileName}}<a class="btn interact-bg gt-p-3" data-tooltip-content="{{ctx.Locale.Tr "repo.commits.view_path"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}


I think $.FileName needs to be escaped to URL parameter. Otherwise C++ or C# breaks the URL.

Yes, likely PathEscape.

-> Improve file history UI and fix URL escaping bug #27531

Follow #27354 Major changes: 1. The `right aligned` in `<th class="one wide right aligned">` is a no-op because it doesn't have any content 2. The `gt-df` in `<td class="sha gt-df">` was wrong, it causes UI misalignment, a table cell shouldn't be "flex" 3. Use `gt-py-0` for `gt-pt-0 gt-pb-0` 4. Simplify the layout for buttons, because the `text right aligned` is widely used and good enough, it doesn't make sense to introduce the `<div class="gt-df gt-je">` 5. Escape the `$.FileName` correctly Before: ![image](https://github.com/go-gitea/gitea/assets/2114189/eb2ced3f-1dad-4149-9ed2-aee4c0663621) After: ![image](https://github.com/go-gitea/gitea/assets/2114189/08244b61-416b-4279-b495-029bc0a96f67)

link to file from its history

59b901b

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 29, 2023

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 29, 2023

github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 29, 2023

denyskon added the type/enhancement An improvement of existing functionality label Sep 29, 2023

denyskon added this to the 1.22.0 milestone Sep 29, 2023

silverwind reviewed Sep 29, 2023

View reviewed changes

templates/repo/commits_list.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Sep 29, 2023

View reviewed changes

templates/repo/commits_list.tmpl Outdated Show resolved Hide resolved

apply code review suggestions

2340028

Co-authored-by: silverwind <me@silverwind.io>

move copy commit sha button to operations

925ad52

pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 29, 2023

delvh reviewed Sep 29, 2023

View reviewed changes

delvh mentioned this pull request Sep 29, 2023

highlight user details link #26998

Merged

Merge remote-tracking branch 'upstream/main' into commit-file-link

70e82d7

remove header title

4ba1f7b

Merge branch 'main' into commit-file-link

41fe333

puni9869 reviewed Sep 30, 2023

View reviewed changes

denyskon added 3 commits October 1, 2023 13:39

ui improvements by silverwind

672393f

Merge branch 'commit-file-link' of ssh://github.com/denyskon/gitea in…

4b97760

…to commit-file-link

Merge remote-tracking branch 'upstream/main' into commit-file-link

a024ef1

delvh approved these changes Oct 1, 2023

View reviewed changes

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 Oct 1, 2023

puni9869 approved these changes Oct 1, 2023

View reviewed changes

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 Oct 1, 2023

techknowlogick enabled auto-merge (squash) October 2, 2023 03:16

techknowlogick added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 2, 2023

Merge branch 'main' into commit-file-link

56bfb59

techknowlogick merged commit 33de64c into go-gitea:main Oct 2, 2023
26 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 2, 2023

denyskon deleted the commit-file-link branch October 2, 2023 05:21

silverwind reviewed Oct 2, 2023

View reviewed changes

wxiaoguang reviewed Oct 2, 2023

View reviewed changes

wxiaoguang mentioned this pull request Oct 9, 2023

Improve file history UI and fix URL escaping bug #27531

Merged

go-gitea locked as resolved and limited conversation to collaborators Dec 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

link to file from its history #27354

link to file from its history #27354

denyskon commented Sep 29, 2023 •

edited

Loading

silverwind commented Sep 29, 2023

silverwind commented Sep 29, 2023

denyskon commented Sep 29, 2023

silverwind commented Sep 29, 2023 •

edited

Loading

denyskon commented Sep 29, 2023

silverwind commented Sep 29, 2023 •

edited

Loading

delvh Sep 29, 2023

silverwind Sep 29, 2023

denyskon Sep 29, 2023

delvh Sep 29, 2023

silverwind Sep 29, 2023

denyskon Sep 29, 2023

silverwind Sep 29, 2023

silverwind commented Sep 29, 2023

denyskon commented Sep 30, 2023

delvh commented Sep 30, 2023

denyskon commented Sep 30, 2023

puni9869 Sep 30, 2023 •

edited

Loading

puni9869 commented Sep 30, 2023

silverwind commented Sep 30, 2023 •

edited

Loading

silverwind commented Sep 30, 2023 •

edited

Loading

wxiaoguang commented Oct 1, 2023

denyskon commented Oct 1, 2023

silverwind commented Oct 2, 2023

silverwind Oct 2, 2023

wxiaoguang Oct 2, 2023

silverwind Oct 2, 2023

wxiaoguang Oct 9, 2023

		<th class="two wide right aligned">{{ctx.Locale.Tr "repo.commits.date"}}</th>
		<th class="one wide right aligned">{{ctx.Locale.Tr "repo.commit.operations"}}</th>

	<th class="one wide right aligned">{{ctx.Locale.Tr "repo.commit.operations"}}</th>
	<th class="one wide right aligned"></th>

	{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}
	{{if $.FileName}}<a class="btn interact-bg" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.view_file"}}" href="{{printf "%s/src/commit/%s/%s" $commitRepoLink (PathEscape .ID.String) $.FileName}}">{{svg "octicon-file-code"}}</a>{{end}}

link to file from its history #27354

link to file from its history #27354

Conversation

denyskon commented Sep 29, 2023 • edited Loading

silverwind commented Sep 29, 2023

silverwind commented Sep 29, 2023

denyskon commented Sep 29, 2023

silverwind commented Sep 29, 2023 • edited Loading

denyskon commented Sep 29, 2023

silverwind commented Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind commented Sep 29, 2023

denyskon commented Sep 30, 2023

delvh commented Sep 30, 2023

denyskon commented Sep 30, 2023

puni9869 Sep 30, 2023 • edited Loading

Choose a reason for hiding this comment

puni9869 commented Sep 30, 2023

silverwind commented Sep 30, 2023 • edited Loading

silverwind commented Sep 30, 2023 • edited Loading

wxiaoguang commented Oct 1, 2023

denyskon commented Oct 1, 2023

silverwind commented Oct 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denyskon commented Sep 29, 2023 •

edited

Loading

silverwind commented Sep 29, 2023 •

edited

Loading

silverwind commented Sep 29, 2023 •

edited

Loading

puni9869 Sep 30, 2023 •

edited

Loading

silverwind commented Sep 30, 2023 •

edited

Loading

silverwind commented Sep 30, 2023 •

edited

Loading