Skip to content

link to file from its history #27354

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

Merged
merged 10 commits into from
Oct 2, 2023
8 changes: 6 additions & 2 deletions templates/repo/commits_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
<tr>
<th class="three wide">{{ctx.Locale.Tr "repo.commits.author"}}</th>
<th class="two wide sha">SHA1</th>
<th class="nine wide message">{{ctx.Locale.Tr "repo.commits.message"}}</th>
<th class="eight wide message">{{ctx.Locale.Tr "repo.commits.message"}}</th>
<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>
Copy link
Member

Choose a reason for hiding this comment

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

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>

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

I guess it looks alright even without the header here.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, looks acceptable:

Screenshot 2023-09-29 at 21 52 42

</tr>
</thead>
<tbody class="commit-list">
Expand All @@ -25,7 +26,6 @@
{{end}}
</td>
<td class="sha gt-df">
<button class="ui button copy-commit-sha gt-df gt-ac" data-clipboard-text="{{.ID}}">{{svg "octicon-copy" 14}}</button>
{{$class := "ui sha label"}}
{{if .Signature}}
{{$class = (print $class " isSigned")}}
Expand Down Expand Up @@ -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}}
Copy link
Member

@puni9869 puni9869 Sep 30, 2023

Choose a reason for hiding this comment

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

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.

</td>
</tr>
{{end}}
</tbody>
Expand Down
7 changes: 0 additions & 7 deletions web_src/css/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -1328,13 +1328,6 @@ img.ui.avatar,
display: inline-block; /* not sure whether it is still needed */
}

.ui .button.copy-commit-sha {
border: 1px solid var(--color-light-border);
margin-right: 3px;
padding: 6px 6px 4px;
background: var(--color-light);
}

.ui .button.truncate {
display: inline-block;
max-width: 100%;
Expand Down
3 changes: 0 additions & 3 deletions web_src/css/repo.css
Original file line number Diff line number Diff line change
Expand Up @@ -3049,9 +3049,6 @@ tbody.commit-list {
.commit-table th.sha {
display: none !important;
}
.commit-table .commit-list .copy-commit-sha {
display: none !important;
}
.comment-header {
flex-wrap: wrap;
}
Expand Down