Skip to content

Add file icons to FileList module#287

Open
janbridley wants to merge 8 commits intomainfrom
add-file-icons
Open

Add file icons to FileList module#287
janbridley wants to merge 8 commits intomainfrom
add-file-icons

Conversation

@janbridley
Copy link
Contributor

@janbridley janbridley commented Jan 5, 2026

Description

The FileList module can now display file icons next to the filenames. This is (in my opinion) a nice feature to have, and opens up a solution for #73, which could use the icons as handles for the file preview popup or as a download button.

Before After
image image

Motivation and Context

Checklist:

@janbridley janbridley marked this pull request as ready for review January 5, 2026 19:53
return "fa-file-excel"
if "powerpoint" in mtype or "presentation" in mtype:
return "fa-file-powerpoint"
if "x-" in mtype or "json" in mtype:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe tighten this up a bit, since x- is pretty broad:

Suggested change
if "x-" in mtype or "json" in mtype:
if mtype.startswith("application/x-") or "json" in mtype:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, on further review, I'm a bit unsure how to handle this -- "x-" is quite broad, but both "application/x-" and "text/x-" are required to handle all common code cases, which ends up falling back to the same behavior as far as I know. We could use something like content-types, but that's an extra dependency. Let me know your thoughts

Copy link
Member

Choose a reason for hiding this comment

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

No big deal. We can go with "x-" in mtype if it works well for your needs. Let's avoid another dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants