Add material icons for file list #26236

wxiaoguang · 2023-07-30T15:53:32Z

Close #11149

Co-authored-by: Yarden Shoham git@yardenshoham.com

yardenshoham

I had to add 4 icons last time to complete the set. I had them in web_src/svg/. You might want them too.

In my implementation, it looked like

Before

After

The files:

material-file.svg

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" class="svg material-file" width="16" height="16" aria-hidden="true"><path fill="#90a4ae" d="M13 9h5.5L13 3.5V9M6 2h8l6 6v12a2 2 0 0 1-2 2H6a2 2 0 0 1-2-2V4c0-1.11.89-2 2-2m5 2H6v16h12v-9h-7V4z"/></svg>

material-folder-root.svg

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" class="svg material-folder-root" width="16" height="16" aria-hidden="true"><path fill="#90a4ae" d="M12 20a8 8 0 0 1-8-8 8 8 0 0 1 8-8 8 8 0 0 1 8 8 8 8 0 0 1-8 8m0-18A10 10 0 0 0 2 12a10 10 0 0 0 10 10 10 10 0 0 0 10-10A10 10 0 0 0 12 2m0 5a5 5 0 0 0-5 5 5 5 0 0 0 5 5 5 5 0 0 0 5-5 5 5 0 0 0-5-5z"/></svg>

material-folder-symlink.svg

<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M10 4H4c-1.11 0-2 .89-2 2v12a2 2 0 002 2h16a2 2 0 002-2V8a2 2 0 00-2-2h-8l-2-2z" fill="#42a5f5" opacity=".745"/><path d="M16.972 10.757v2.641h-6.561v5.281h6.561v2.641l6.562-5.281-6.562-5.282z" opacity=".81" fill="#c5e5fd"/></svg>

material-folder.svg

<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M10 4H4c-1.11 0-2 .89-2 2v12a2 2 0 002 2h16a2 2 0 002-2V8a2 2 0 00-2-2h-8l-2-2z" fill="#42a5f5"/></svg>

modules/fileicon/basic.go

delvh · 2023-07-30T17:25:38Z

What I haven't quite understood yet: What's the difference to #24147?
Wasn't its main problem that it should be configurable?

yardenshoham · 2023-07-30T17:27:49Z

Wasn't its main problem that it should be configurable?

I think here you can untar, change files, tar again.

yardenshoham · 2023-07-30T17:30:44Z

Something bad happened here

I remember that what fixed that for me was passing the SVGs through build/generate-svg.js's processFile

silverwind · 2023-07-30T22:16:01Z

I think I prefer octicons for unrecognized folders and files and a option could be added that enables the file type icons. I personally would prefer for folders to not receive any custom icons.

lunny · 2023-07-31T01:07:02Z

Could we have a configuration to allow users to switch back to Basic from the default Material? Then I will give my L-G-T-M

puni9869 · 2023-07-31T02:44:01Z

Will it be the default look of icon in 1.21
Its looks good.

wxiaoguang · 2023-07-31T02:51:41Z

What I haven't quite understood yet: What's the difference to #24147? Wasn't its main problem that it should be configurable?

The differences:

This PR uses LFS to store the icon pack file (.tgz), so only 300+ lines (+436 −75) are added (the original one added more than 6000 lines)
- The LFS could also benefit other large file storing in Gitea's git repo in the future, to avoid bloating the main code base.
The external icon pack doesn't go into Gitea's image assets, then they can be modified/removed independently.
End users can make/use/publish their own icon pack
- It's possible to support more "plug-able pack files"

JakobDev · 2023-07-31T06:16:43Z

Sound code, but does this work with different styles? It is possible to select a style in Gitea. Does the Icons look good on every style?

wxiaoguang · 2023-07-31T06:45:25Z

Sound code, but does this work with different styles? It is possible to select a style in Gitea. Does the Icons look good on every style?

The quick is answer is: it might not work that well. Because there are some "light" icons in the icon set, but they are not used yet. But I think it's not in this PR's scope. Due to how Gitea's backend/frontend work, it needs a lot of more time to fix it.

…c for folder

wxiaoguang · 2023-07-31T07:08:04Z

@yardenshoham

I had to add 4 icons last time to complete the set. I had them in web_src/svg/. You might want them too.
In my implementation, it looked like .....

If I understand correctly, only "material-folder(-generic).svg" and "material-folder-symlink.svg" are used, now they are there.

Something bad happened here
I remember that what fixed that for me was passing the SVGs through build/generate-svg.js's processFile

There were also a lot of backend code handling SVG contents, which were fragile. Now I have rewritten them (and add tests).

And I also added a temporary option [ui].FILE_ICON_THEME , in case some edge versions users would like to revert to the old icon theme.

I think we can introduce an "undocumented" config option in this PR to set "basic" or "material" icon set, then replace the option with user setting in next PR soon.
Since 1.21 is still at early stage, the undocumented option won't cause end-user problems IMO.

yardenshoham · 2023-07-31T07:49:05Z

The icons look a bit small and too high

delvh

Works.

yardenshoham · 2023-08-01T12:23:57Z

@silverwind do you want to block?

delvh · 2023-08-02T19:55:37Z

So, to summarize the discussion on this page:

It is a must that the icon pack is configurable. We don't care (for now) how easy it is, as long as it is possible.
the possible icon pack management tools are
1. include one zipped file (i.e. tar.gz) containing all icons, requires LFS
2. include one zipped file without LFS, so as a binary where every change copies the whole binary
3. include a folder containing all icons, will bring the changes of this PR into the hundreds
4. include an npm dependency, removes customizability but improves maintainability
5. offer a download URL for i.e. dl.gitea.com to download (additional) icon packs from
6. (did I forget one?)

As I see it, 3. is the only one whose disadvantage is limited to this PR instead of the future maintainability of Gitea or the downloadability of the Gitea repo.
Everything else will introduce a drawback we cannot get rid of anymore.
To be even more maintainable, we can add a make target to update our icon pack to a new state.
To be even more customizable, we can offer a download URL to download additional packs from in the future.

yardenshoham · 2023-08-02T21:39:57Z

How do you customize with option 3?

silverwind · 2023-08-02T21:43:29Z

I still don't like the introduction of LFS for this, it means the gitea repo can no longer be hosted on a default installation of gitea which has LFS disabled. 294kB is certainly not a size where I would even consider LFS, that starts in the realms of 100MB+ for me.

Not going to block if everyone else agrees to LFS, but I don't.

delvh · 2023-08-02T21:46:26Z

How do you customize with option 3?

Isn't that even the easiest one to customize?
You can simply add/change/remove files in the directory to customize them?
That of course requires that reading them in is dynamically instead of hardcoded.

yardenshoham · 2023-08-03T06:15:46Z

Gotcha, so it'll go on public. Yeah, let's do that.

wxiaoguang · 2023-08-03T06:17:20Z

I prefer LFS than bloating the codebase with hundreds (maybe thousands in the future) vendoring files.

lunny · 2023-08-03T07:26:00Z

I still don't like the introduction of LFS for this, it means the gitea repo can no longer be hosted on a default installation of gitea which has LFS disabled. 294kB is certainly not a size where I would even consider LFS, that starts in the realms of 100MB+ for me.

Not going to block if everyone else agrees to LFS, but I don't.

Maybe we could enable LFS default now. Because it will not require any extra installation or configuration for users.

JakobDev · 2023-08-03T07:35:14Z

Because it will not require any extra installation or configuration for users.

I think you need git LFS installed. LFS is not included in git by default.

lunny · 2023-08-03T07:36:26Z

Because it will not require any extra installation or configuration for users.

I think you need git LFS installed. LFS is not included in git by default.

Don't need. Only git client need git-LFS. Gitea's LFS logic is written by Golang and it's internal.

delvh · 2023-08-03T07:39:35Z

Isn't that the whole point of that argument?
We are not talking about Gitea but its users/developers.

lunny · 2023-08-03T08:04:43Z

Isn't that the whole point of that argument? We are not talking about Gitea but its users/developers.

For developers, they already need to install git/nodejs/golang/python, I don't think installing git-lfs is difficult for them.

KN4CK3R · 2023-08-03T08:06:56Z

I don't understand why we should want these icons in our codebase and inherit the burden of keeping them updated if we could rely on external icon packs.

Edit: Gitlab for example manages them in a separate repo and still they just download an external source when building with CI.

denyskon · 2023-08-03T08:19:36Z

I also don't think we need to store it in the codebase. Why not just download the icons when building?

lunny · 2023-08-03T08:31:46Z

I also don't think we need to store it in the codebase. Why not just download the icons when building?

Or download the icons when using. But it's more difficult. We need to define an icon package or repository specification.

silverwind · 2023-08-04T13:54:55Z

I do think keeping build artifacts in the repo is beneficial so gitea can be built without internet access (minus go and npm dependencies which of course require it), we do that for numerous dependencies like Fomantic, the SVGs, etc.

silverwind · 2023-08-04T13:57:32Z

We should add https://www.npmjs.com/package/material-icon-theme as a npm dependency, and build from node_modules/material-icon-theme, this is preferable to the current download script anyways because certain environments have private npm registries and such where registry.npmjs.org is intentionally unreachable and only the private registry is reachable.

By offloading the download to the npm client, we get this compatibilty for free, in addtion to having the dependency version-locked.

denyskon · 2023-08-04T16:48:21Z

I think npm is the best option, even though we lose customizability. I don't think that many users will really want install a different icon theme.....

delvh · 2023-08-04T17:00:35Z

Do we need a survey, or is there a consensus on how to move forward?

yardenshoham · 2023-08-04T17:09:24Z

If there's no need for customizability we could have landed this months ago

silverwind · 2023-08-04T17:13:19Z

We don't loose customizability just by changing the way the material icons are fetched.

We just need to formalize a spec format for a icon pack, e.g. that it should contain a JSON mapping of file name or path to icons and the icons in a subdirectory as SVGs that reference the mapping values.

By the way, I find zip files more easy to handle than tgz, especially on Windows, so likely we should move to that format.

wxiaoguang · 2023-08-04T17:17:21Z

I have no motivation for making more changes because I do not like these colorful icons. I wrote this PR just because I promised.

While I do not think #24147 is good enough, eg: the packing was wrong (the material-icons.json shouldn't be assumed to be a local file or read by os.ReadFile)

And, mixing the external assets into Gitea's SVG assets doesn't look good to me.

Feel free to replace this PR, I would have no objection if the new solution is maintainable and satisfies:

Not bloating the codebase too much (600+ files => 800+ files => maybe soon 1000+ files? And the google official material repo has more than 10K src files)
Not mixing the external assets into the Gitea's own assets (IMO Gitea's internal SVG system should only use its managed SVG icons, but not depend on these auto-updated external icons)

Or, let the TOC team decide.

yardenshoham · 2023-08-04T17:26:46Z

Thanks @wxiaoguang, it was worth a shot

wxiaoguang · 2025-03-09T17:26:19Z

A new PR:

-> Add material icons for file list #33837

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

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 30, 2023

wxiaoguang added this to the 1.21.0 milestone Jul 30, 2023

wxiaoguang changed the title ~~Add matrial icons for file list~~ Add material icons for file list Jul 30, 2023

wxiaoguang force-pushed the custom-file-icon branch 5 times, most recently from dd2287f to 59352fb Compare July 30, 2023 16:22

file icon

c502126

wxiaoguang force-pushed the custom-file-icon branch from 59352fb to c502126 Compare July 30, 2023 16:28

yardenshoham reviewed Jul 30, 2023

View reviewed changes

modules/fileicon/basic.go Outdated Show resolved Hide resolved

yardenshoham added type/feature Completely new functionality. Can only be merged if feature freeze is not active. topic/ui Change the appearance of the Gitea UI labels Jul 30, 2023

wxiaoguang added 2 commits July 31, 2023 14:49

normalizes SVG images when loading

4945914

add config option FILE_ICON_THEME, use builtin material-folder-generi…

10b2d48

…c for folder

pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 31, 2023

Merge branch 'main' into custom-file-icon

4a7dbe9

delvh approved these changes Aug 1, 2023

View reviewed changes

wxiaoguang closed this Aug 4, 2023

wxiaoguang mentioned this pull request Aug 4, 2023

Refactor backend SVG package and add tests #26335

Merged

GiteaBot removed this from the 1.21.0 milestone Aug 5, 2023

go-gitea locked as resolved and limited conversation to collaborators Nov 2, 2023

wxiaoguang deleted the custom-file-icon branch March 9, 2025 17:24

Add material icons for file list #26236

Add material icons for file list #26236

Conversation

wxiaoguang commented Jul 30, 2023 • edited Loading

yardenshoham left a comment • edited Loading

Choose a reason for hiding this comment

Before

After

material-file.svg

material-folder-root.svg

material-folder-symlink.svg

material-folder.svg

delvh commented Jul 30, 2023

yardenshoham commented Jul 30, 2023

yardenshoham commented Jul 30, 2023

silverwind commented Jul 30, 2023

lunny commented Jul 31, 2023

puni9869 commented Jul 31, 2023

wxiaoguang commented Jul 31, 2023 • edited Loading

JakobDev commented Jul 31, 2023

wxiaoguang commented Jul 31, 2023

wxiaoguang commented Jul 31, 2023 • edited Loading

yardenshoham commented Jul 31, 2023

delvh left a comment

Choose a reason for hiding this comment

yardenshoham commented Aug 1, 2023

delvh commented Aug 2, 2023

yardenshoham commented Aug 2, 2023

silverwind commented Aug 2, 2023 • edited Loading

delvh commented Aug 2, 2023

yardenshoham commented Aug 3, 2023

wxiaoguang commented Aug 3, 2023

lunny commented Aug 3, 2023

JakobDev commented Aug 3, 2023

lunny commented Aug 3, 2023

delvh commented Aug 3, 2023

lunny commented Aug 3, 2023 • edited Loading

KN4CK3R commented Aug 3, 2023 • edited Loading

denyskon commented Aug 3, 2023

lunny commented Aug 3, 2023

silverwind commented Aug 4, 2023 • edited Loading

silverwind commented Aug 4, 2023 • edited Loading

denyskon commented Aug 4, 2023

delvh commented Aug 4, 2023

yardenshoham commented Aug 4, 2023

silverwind commented Aug 4, 2023 • edited Loading

wxiaoguang commented Aug 4, 2023 • edited Loading

yardenshoham commented Aug 4, 2023

wxiaoguang commented Mar 9, 2025

wxiaoguang commented Jul 30, 2023 •

edited

Loading

yardenshoham left a comment •

edited

Loading

wxiaoguang commented Jul 31, 2023 •

edited

Loading

wxiaoguang commented Jul 31, 2023 •

edited

Loading

silverwind commented Aug 2, 2023 •

edited

Loading

lunny commented Aug 3, 2023 •

edited

Loading

KN4CK3R commented Aug 3, 2023 •

edited

Loading

silverwind commented Aug 4, 2023 •

edited

Loading

silverwind commented Aug 4, 2023 •

edited

Loading

silverwind commented Aug 4, 2023 •

edited

Loading

wxiaoguang commented Aug 4, 2023 •

edited

Loading