Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Chore: migrate dashboard-icons #4564

Merged
merged 4 commits into from
Jan 5, 2025
Merged

Chore: migrate dashboard-icons #4564

merged 4 commits into from
Jan 5, 2025

Conversation

walkxcode
Copy link
Contributor

Info: homarr-labs/dashboard-icons#797 (comment).

Note: The walkxcode/dashboard-icons repository will continue to function indefinitely but may no longer receive updates.

@walkxcode walkxcode marked this pull request as ready for review January 5, 2025 21:07
@shamoon shamoon changed the title Migrate dashboard-icons to homarr-labs Chore: migrate dashboard-icons to homarr-labs Jan 5, 2025
Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

walkxcode himself, cool.

Well thanks for taking the time to PR, thanks for your project.

I think this contains an error at the moment, it's re-declaring the const and the (unintentionally?) added webp block is unreachable

@walkxcode
Copy link
Contributor Author

walkxcode himself, cool.

Well thanks for taking the time to PR, thanks for your project.

I think this contains an error at the moment, it's re-declaring the const and the (unintentionally?) added webp block is unreachable

The constant uses the same logic as the SVG and PNG retrieval. Folder links to jsDelivr show as unreachable because we’ve exceeded the ‘package’ limit. However, direct file links, such as https://cdn.jsdelivr.net/gh/homarr-labs/dashboard-icons/webp/1password.webp, will still work.

@walkxcode
Copy link
Contributor Author

Ah seems like we're missing the if (icon.endsWith(".webp")) {!

@walkxcode
Copy link
Contributor Author

Ideally the PNGs should be removed completely as they have the same function as the WebPs but with a bigger size, but that's all up to you ofcourse. Let me know if you want the WebPs or PNGs :)

@shamoon
Copy link
Collaborator

shamoon commented Jan 5, 2025

Thanks. Yea we can make the fallback to webp, its support is pretty darn good at this point

@shamoon
Copy link
Collaborator

shamoon commented Jan 5, 2025

Actually I take that back. That would be breaking or at least kinda confusing. You can add webp if you like but please leave the png for now

@walkxcode walkxcode requested a review from shamoon January 5, 2025 21:34
@shamoon shamoon enabled auto-merge (squash) January 5, 2025 21:41
Copy link
Collaborator

@shamoon shamoon left a comment

Choose a reason for hiding this comment

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

Thanks again

@shamoon shamoon merged commit 6a0fbba into gethomepage:dev Jan 5, 2025
5 checks passed
@shamoon shamoon changed the title Chore: migrate dashboard-icons to homarr-labs Chore: migrate dashboard-icons Jan 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants