Skip to content

Improve issue card #26561

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Improve issue card #26561

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3fd3b8f
improve
yp05327 Aug 17, 2023
b0e002f
fix lint
yp05327 Aug 17, 2023
3b23b2a
Update templates/repo/issue/card.tmpl
yp05327 Aug 17, 2023
ebeb170
Merge branch 'main' into improve-issue-card
yp05327 Oct 23, 2023
45220b9
Merge branch 'main' into improve-issue-card
yp05327 Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions routers/web/repo/projects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func Projects(ctx *context.Context) {

ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects)
ctx.Data["IsShowClosed"] = isShowClosed
ctx.Data["IsProjectsPage"] = true
ctx.Data["PageIsProjects"] = true
ctx.Data["SortType"] = sortType

ctx.HTML(http.StatusOK, tplProjects)
Expand Down Expand Up @@ -362,7 +362,7 @@ func ViewProject(ctx *context.Context) {
return
}

ctx.Data["IsProjectsPage"] = true
ctx.Data["PageIsProjects"] = true
ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects)
ctx.Data["Project"] = project
ctx.Data["IssuesMap"] = issuesMap
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/header.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
{{end}}

{{if and (not .UnitProjectsGlobalDisabled) (.Permission.CanRead $.UnitTypeProjects)}}
<a href="{{.RepoLink}}/projects" class="{{if .IsProjectsPage}}active {{end}}item">
<a href="{{.RepoLink}}/projects" class="{{if .PageIsProjects}}active {{end}}item">
{{svg "octicon-project"}} {{.locale.Tr "repo.project_board"}}
{{if .Repository.NumOpenProjects}}
<span class="ui small label">{{CountFmt .Repository.NumOpenProjects}}</span>
Expand Down
32 changes: 17 additions & 15 deletions templates/repo/issue/card.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{{with .Issue}}
{{if eq $.Page.Project.CardType 1}}{{/* Images and Text*/}}
<div class="card-attachment-images">
<div class="issue-card-content card-attachment-images">
{{range (index $.Page.issuesAttachmentMap .ID)}}
<img src="{{.DownloadURL}}" alt="{{.Name}}" />
{{end}}
</div>
{{end}}
<div class="content gt-p-0 gt-w-100">
<div class="issue-card-content">
<div class="gt-df gt-items-start">
<div class="issue-card-icon">
{{template "shared/issueicon" .}}
Expand All @@ -18,7 +18,7 @@
</a>
{{end}}
</div>
<div class="meta gt-my-2">
<div class="gt-my-2">
<span class="text light grey muted-links">
{{if not $.Page.Repository}}{{.Repo.FullName}}{{end}}#{{.Index}}
{{$timeStr := TimeSinceUnix .GetLastEventTimestamp ctx.Locale}}
Expand All @@ -32,16 +32,16 @@
</span>
</div>
{{if .MilestoneID}}
<div class="meta gt-my-2">
<a class="milestone" href="{{$.Page.RepoLink}}/milestone/{{.MilestoneID}}">
<div class="gt-df gt-my-2">
<a class="sidebar-item-link muted" href="{{$.Page.RepoLink}}/milestone/{{.MilestoneID}}">
{{svg "octicon-milestone" 16 "gt-mr-2 gt-vm"}}
<span class="gt-vm">{{.Milestone.Name}}</span>
{{.Milestone.Name}}
</a>
</div>
{{end}}
{{if $.Page.LinkedPRs}}
{{range index $.Page.LinkedPRs .ID}}
<div class="meta gt-my-2">
<div class="gt-my-2">
<a href="{{$.Page.RepoLink}}/pulls/{{.Index}}">
<span class="gt-m-0 text {{if .PullRequest.HasMerged}}purple{{else if .IsClosed}}red{{else}}green{{end}}">{{svg "octicon-git-merge" 16 "gt-mr-2 gt-vm"}}</span>
<span class="gt-vm">{{.Title}} <span class="text light grey">#{{.Index}}</span></span>
Expand All @@ -51,16 +51,18 @@
{{end}}
</div>

{{if or .Labels .Assignees}}
<div class="extra content labels-list gt-p-0 gt-pt-2">
{{if .Labels}}
<div class="issue-card-content labels-list">
{{range .Labels}}
<a target="_blank" href="{{$.Page.RepoLink}}/issues?labels={{.ID}}">{{RenderLabel ctx .}}</a>
<a {{if $.Page.PageIsProjects}}target="_blank"{{end}} href="{{$.Page.RepoLink}}/issues?labels={{.ID}}">{{RenderLabel ctx .}}</a>
{{end}}
Copy link
Member

Choose a reason for hiding this comment

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

{{if $.Page.PageIsProjects}} should it be {{if .PageIsProjects}} 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Not possible in this case.
Go templates are weird when it comes to range:
What is happening here is implicitly a

for _, . := range(Labels) { … }

, with . being the base variable the Go templates can use.
In other words, all previous variables get removed leaving only the properties of a label for a whole iteration.
That's why you need to reference a template global variable by prefixing it with $.

Copy link
Contributor

Choose a reason for hiding this comment

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

ps: in other places, the "Page" variable is usually called "root" or "ctxData", which means the root context data of this request (aka: ctx.Data in backend code)

</div>
{{end}}
{{if .Assignees}}
<div class="issue-card-content gt-df">
{{range .Assignees}}
<a class="flex-text-inline" {{if $.Page.PageIsProjects}}target="_blank"{{end}} href="{{.HomeLink}}" data-tooltip-content="{{ctx.Locale.Tr "repo.projects.column.assigned_to"}} {{.Name}}">{{ctx.AvatarUtils.Avatar . 28 "mini gt-mr-3"}}</a>
{{end}}
<div class="right floated">
{{range .Assignees}}
<a target="_blank" href="{{.HomeLink}}" data-tooltip-content="{{ctx.Locale.Tr "repo.projects.column.assigned_to"}} {{.Name}}">{{ctx.AvatarUtils.Avatar . 28 "mini gt-mr-3"}}</a>
{{end}}
</div>
</div>
{{end}}
{{end}}
9 changes: 8 additions & 1 deletion web_src/css/features/projects.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
}

.card-attachment-images {
display: inline-block;
display: flex;
flex-wrap: wrap;
white-space: nowrap;
overflow: hidden;
text-align: center;
Expand All @@ -82,12 +83,18 @@
.card-attachment-images img {
display: inline-block;
max-height: 50px;
max-width: 100%;
border-radius: var(--border-radius);
margin-right: 2px;
Copy link
Member

@silverwind silverwind Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
margin-right: 2px;
margin-right: 2px;
aspect-ratio: 1;

Better to have this just in case, to ensure the image maintains aspect ratio when scaling width/height.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this overlap be a problem?

https://jsfiddle.net/fmapw0ye/

image

Copy link
Member

@silverwind silverwind Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This will fix that overlap, does require max-height on the parent. Better would be direct max-height on the img without any parent.

max-width: 100%;
max-height: 100%;
aspect-ratio: 1;

}

.card-attachment-images img:not(:first-child) {
padding-top: 0.25rem;
}

.card-attachment-images img:only-child {
max-height: 90px;
max-width: 100%;
margin: auto;
}

Expand Down
8 changes: 8 additions & 0 deletions web_src/css/repo/issue-card.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
background: var(--color-card);
}

.issue-card .issue-card-content {
width: 100%;
}

.issue-card .issue-card-content:not(:first-child) {
padding-top: 0.25rem;
}

.issue-card-icon,
.issue-card-unpin {
margin-top: 1px;
Expand Down