Also show file-specific icons in the PR file tree #33921

kerwin612 · 2025-03-17T11:16:20Z

Fix #33914

before:

after:

kerwin612 · 2025-03-18T07:16:58Z

file diff

before:

after:

lutinglt · 2025-03-18T15:41:17Z

Perhaps it would be better to use Material Icons for the folder as well?
The Material Icons for GitHub browser plugin follows this logic

kerwin612 · 2025-03-19T02:55:08Z

Perhaps it would be better to use Material Icons for the folder as well? The Material Icons for GitHub browser plugin follows this logic

This commit [888296b] is aimed at unifying the icons of folders on the UI.

before:

after:

services/repository/files/tree_test.go

templates/repo/view_list.tmpl

routers/web/repo/treelist.go

wxiaoguang · 2025-03-19T14:55:43Z

routers/web/repo/treelist.go

+		entry, _ := headCommit.GetTreeEntryByPath(file.HeadPath)
+		if entry == nil {
+			entry, _ = baseCommit.GetTreeEntryByPath(file.HeadPath)
+		}


Do we really need this extra call to collect information?

The diff tree file should already have enough information: dir or not, submodule or not, etc.

After carefully reviewing the current code, I found that the file icons on the file list page are derived from RenderUtils.RenderFileIcon:

gitea/templates/repo/view_list.tmpl

Line 18 in d3c7992

{{ctx.RenderUtils.RenderFileIcon $entry}}

gitea/modules/templates/util_render.go

Line 191 in d3c7992

func (ut *RenderUtils) RenderFileIcon(entry *git.TreeEntry) template.HTML {

To standardize the global file icons, we should unify their usage to RenderUtils.RenderFileIcon. Since this method requires a parameter of type TreeEntry, we need to obtain the entry object here.
I initially attempted to simplify the RenderFileIcon function by using only entryMode to retrieve the icon. However, after tracing and analysis, it was found that this approach is not feasible. The function's logic relies on multiple fields, making it more appropriate to use the entry object directly.

Abstraction, if the diff tree files already provide enough information you need.

The diff-files only provides name, isSubmodule, isViewed, status:

gitea/routers/web/repo/treelist.go

Lines 62 to 69 in d98cae2

type FileDiffFile struct {

Name string

NameHash string

FileIcon template.HTML

IsSubmodule bool

IsViewed bool

Status string

}

gitea/services/gitdiff/git_diff_tree.go

Lines 22 to 35 in d98cae2

type DiffTreeRecord struct {

// Status is one of 'added', 'deleted', 'modified', 'renamed', 'copied', 'typechanged', 'unmerged', 'unknown'

Status string

// For renames and copies, the percentage of similarity between the source and target of the move/rename.

Score uint8

HeadPath string

BasePath string

HeadMode git.EntryMode

BaseMode git.EntryMode

HeadBlobID string

BaseBlobID string

}

Meanwhile, RenderFileIcon involves entry.Name(), entry.entryMode(), and entry.FollowLink():

gitea/modules/fileicon/basic.go

Lines 24 to 38 in d98cae2

func BasicThemeIcon(entry *git.TreeEntry) template.HTML {

svgName := "octicon-file"

switch {

case entry.IsLink():

svgName = "octicon-file-symlink-file"

if te, err := entry.FollowLink(); err == nil && te.IsDir() {

svgName = "octicon-file-directory-symlink"

}

case entry.IsDir():

svgName = BasicThemeFolderIconName(false)

case entry.IsSubModule():

svgName = "octicon-file-submodule"

}

return svg.RenderHTML(svgName)

}

gitea/modules/fileicon/material.go

Lines 92 to 123 in d98cae2

func (m *MaterialIconProvider) FileIcon(ctx reqctx.RequestContext, entry *git.TreeEntry) template.HTML {

if m.rules == nil {

return BasicThemeIcon(entry)

}

if entry.IsLink() {

if te, err := entry.FollowLink(); err == nil && te.IsDir() {

// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work

return svg.RenderHTML("material-folder-symlink", 16, "octicon-file-directory-symlink")

}

return svg.RenderHTML("octicon-file-symlink-file") // TODO: find some better icons for them

}

name := m.findIconNameByGit(entry)

if name == "folder" {

// the material icon pack's "folder" icon doesn't look good, so use our built-in one

// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work

return m.FolderIcon(ctx, false)

}

if iconSVG, ok := m.svgs[name]; ok && iconSVG != "" {

// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work

extraClass := "octicon-file"

switch {

case entry.IsDir():

extraClass = BasicThemeFolderIconName(false)

case entry.IsSubModule():

extraClass = "octicon-file-submodule"

}

return m.renderFileIconSVG(ctx, name, iconSVG, extraClass)

}

return svg.RenderHTML("octicon-file")

}

gitea/modules/fileicon/material.go

Line 98 in d98cae2

if te, err := entry.FollowLink(); err == nil && te.IsDir() {

The diff-files only provides name, isSubmodule, isViewed, status:

Because current "diff tree list" only contains files. You could know whether it is a link by the diff tree item, and you don't need to know whether it is a dir or need to follow link, the frontend doesn't support it.

Could you please take another look at this commit to see if it aligns with your expectations for an appropriate solution?
4db35ba

wxiaoguang · 2025-03-19T14:56:03Z

templates/repo/view.tmpl

+<script type="module">
+window.config.pageData.folderIcon = {{ctx.RenderUtils.RenderFolderIcon false}};
+window.config.pageData.openFolderIcon = {{ctx.RenderUtils.RenderFolderIcon true}};
+</script>


Why .........

As mentioned earlier, in order to unify the icon sources (you can refer to the screenshots in the PR description. The "before" screenshot shows the current situation, and the "after" screenshot shows the situation after the modification, where the icon styles of folders have also been unified).

It is precisely this modification that enables the unification of folder icons.

I know you are trying to "unify the icon sources"

But why inline scripts?

Since this part involves the integration of templates and Vue components, and the icons are used within the Vue components, but RenderFileIcon cannot be directly used in the Vue components.
Therefore, i follow the common practice widely adopted in the current project:
① First, obtain the icon values in the template, and then cache them using pageData.
② Then, retrieve the icons from pageData in the Vue component.

gitea/web_src/js/components/ViewFileTreeItem.vue

Line 91 in d98cae2

<span v-else-if="isDirectory && !collapsed" class="item-icon" v-html="pageData.openFolderIcon"/>

I do not think it is right.

You need to provide the open/closed icons for each entry item

Currently, this is truly the only implementation method that comes to my mind. 😅

services/repository/files/tree.go

Optimize File Icons in file-tree to Match Those in file-list

Loading
Loading status checks…

53b5833

GiteaBot added the lgtm/need 2 label Mar 17, 2025

pull-request-size bot added the size/M label Mar 17, 2025

github-actions bot added modifies/go modifies/frontend labels Mar 17, 2025

kerwin612 added 2 commits March 18, 2025 14:07

Merge branch 'main' into optimize-filetree-icons-match

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

a1a0bd7

on the diff page also remains consistent

Loading
Loading status checks…

b478654

pull-request-size bot added size/L and removed size/M labels Mar 18, 2025

lunny changed the title ~~Optimize File Icons in file-tree to Match Those in file-list~~ Optimize File Icons in file-tree to Match Those in file-list and also in file-tree in pull request changed files. Mar 18, 2025

lunny changed the title ~~Optimize File Icons in file-tree to Match Those in file-list and also in file-tree in pull request changed files.~~ Optimize file icons in file-tree to match those in file-list and also in file-tree of pull request changed files. Mar 18, 2025

kerwin612 added 2 commits March 19, 2025 08:24

Merge branch 'main' into optimize-filetree-icons-match

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

d05e06e

fix

Loading
Loading status checks…

888296b

github-actions bot added the modifies/templates label Mar 19, 2025

Merge branch 'main' into optimize-filetree-icons-match

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

c834223

fix lint error

Loading
Loading status checks…

451a37c

lunny added this to the 1.24.0 milestone Mar 19, 2025

lunny added type/bug topic/ui skip-changelog labels Mar 19, 2025

kerwin612 added 3 commits March 19, 2025 05:54

fix tests

Loading
Loading status checks…

9269751

fix tests

Loading
Loading status checks…

68d9e6b

fix tests

Loading
Loading status checks…

e42e4f9

delvh changed the title ~~Optimize file icons in file-tree to match those in file-list and also in file-tree of pull request changed files.~~ Also show file-specific icons in the PR file tree Mar 19, 2025

wxiaoguang reviewed Mar 19, 2025

View reviewed changes

services/repository/files/tree_test.go Outdated Show resolved Hide resolved

wxiaoguang reviewed Mar 19, 2025

View reviewed changes

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

wxiaoguang reviewed Mar 19, 2025

View reviewed changes

routers/web/repo/treelist.go Outdated Show resolved Hide resolved

wxiaoguang reviewed Mar 19, 2025

View reviewed changes

fix

Loading
Loading status checks…

d3c7992

wxiaoguang reviewed Mar 20, 2025

View reviewed changes

services/repository/files/tree.go Outdated Show resolved Hide resolved

fix

Loading
Loading status checks…

d98cae2

wxiaoguang reviewed Mar 20, 2025

View reviewed changes

services/repository/files/tree.go Outdated Show resolved Hide resolved

kerwin612 added 7 commits March 20, 2025 03:21

fix

Loading
Loading status checks…

a8f6d18

fix

Loading
Loading status checks…

4145d5d

fix

Loading
Loading status checks…

4db35ba

fix

Loading
Loading status checks…

9ce6848

Merge branch 'main' into optimize-filetree-icons-match

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

dfdc3ea

fix

Loading
Loading status checks…

5587b9d

fix

Loading
Loading status checks…

f1f786a

kerwin612 requested a review from wxiaoguang March 21, 2025 01:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Also show file-specific icons in the PR file tree #33921

Also show file-specific icons in the PR file tree #33921

kerwin612 commented Mar 17, 2025 •

edited by lunny

Loading

kerwin612 commented Mar 18, 2025

lutinglt commented Mar 18, 2025

kerwin612 commented Mar 19, 2025

wxiaoguang Mar 19, 2025

kerwin612 Mar 20, 2025

wxiaoguang Mar 20, 2025

kerwin612 Mar 20, 2025

wxiaoguang Mar 20, 2025

kerwin612 Mar 20, 2025

wxiaoguang Mar 19, 2025

kerwin612 Mar 20, 2025

wxiaoguang Mar 20, 2025

kerwin612 Mar 20, 2025

wxiaoguang Mar 20, 2025

kerwin612 Mar 20, 2025

	type FileDiffFile struct {
	Name string
	NameHash string
	FileIcon template.HTML
	IsSubmodule bool
	IsViewed bool
	Status string
	}

	type DiffTreeRecord struct {
	// Status is one of 'added', 'deleted', 'modified', 'renamed', 'copied', 'typechanged', 'unmerged', 'unknown'
	Status string

	// For renames and copies, the percentage of similarity between the source and target of the move/rename.
	Score uint8

	HeadPath string
	BasePath string
	HeadMode git.EntryMode
	BaseMode git.EntryMode
	HeadBlobID string
	BaseBlobID string
	}

	func BasicThemeIcon(entry *git.TreeEntry) template.HTML {
	svgName := "octicon-file"
	switch {
	case entry.IsLink():
	svgName = "octicon-file-symlink-file"
	if te, err := entry.FollowLink(); err == nil && te.IsDir() {
	svgName = "octicon-file-directory-symlink"
	}
	case entry.IsDir():
	svgName = BasicThemeFolderIconName(false)
	case entry.IsSubModule():
	svgName = "octicon-file-submodule"
	}
	return svg.RenderHTML(svgName)
	}

	func (m MaterialIconProvider) FileIcon(ctx reqctx.RequestContext, entry git.TreeEntry) template.HTML {
	if m.rules == nil {
	return BasicThemeIcon(entry)
	}

	if entry.IsLink() {
	if te, err := entry.FollowLink(); err == nil && te.IsDir() {
	// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work
	return svg.RenderHTML("material-folder-symlink", 16, "octicon-file-directory-symlink")
	}
	return svg.RenderHTML("octicon-file-symlink-file") // TODO: find some better icons for them
	}

	name := m.findIconNameByGit(entry)
	if name == "folder" {
	// the material icon pack's "folder" icon doesn't look good, so use our built-in one
	// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work
	return m.FolderIcon(ctx, false)
	}
	if iconSVG, ok := m.svgs[name]; ok && iconSVG != "" {
	// keep the old "octicon-xxx" class name to make some "theme plugin selector" could still work
	extraClass := "octicon-file"
	switch {
	case entry.IsDir():
	extraClass = BasicThemeFolderIconName(false)
	case entry.IsSubModule():
	extraClass = "octicon-file-submodule"
	}
	return m.renderFileIconSVG(ctx, name, iconSVG, extraClass)
	}
	return svg.RenderHTML("octicon-file")
	}

Also show file-specific icons in the PR file tree #33921

Are you sure you want to change the base?

Also show file-specific icons in the PR file tree #33921

Conversation

kerwin612 commented Mar 17, 2025 • edited by lunny Loading

kerwin612 commented Mar 18, 2025

lutinglt commented Mar 18, 2025

kerwin612 commented Mar 19, 2025

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

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

kerwin612 commented Mar 17, 2025 •

edited by lunny

Loading