Fix formatting table icons #1016
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tunetheweb
reviewed
Feb 24, 2025
Co-authored-by: Barry Pollard <[email protected]>
max-ostapenko
requested changes
Feb 24, 2025
src/js/techreport/tableLinked.js
Outdated
| @@ -96,12 +95,13 @@ class TableLinked { | |||
| cell.classList.add('app-cell'); | |||
|
|
|||
| const img = document.createElement('span'); | |||
| const imgUrl = `https://cdn.httparchive.org/static/icons/${formattedApp}.png`; | |||
| const imgUrl = `https://cdn.httparchive.org/static/icons/${encodeURI(app)}.png`; | |||
There was a problem hiding this comment.
@sarahfossheim please use the icon name from the technology data.
It is not always equal to technology name.
There was a problem hiding this comment.
@max-ostapenko I see the example you linked to ends with .svg, does this mean the file format is expected to come from there as well? Because not all of them seem to have the correct format.
For example:
{
"technology": "1C-Bitrix",
"category": "CMS, Ecommerce",
"description": "1C-Bitrix is a system of web project management, universal software for the creation, support and successful development of corporate websites and online stores.",
"icon": "1C-Bitrix.svg",
"origins": {
"mobile": 85308,
"desktop": 58771
}
}
"icon": "1C-Bitrix.svg" would turn into:
https://cdn.httparchive.org/static/icons/1C-Bitrix.svg
But that gives the following error:
<Error>
<Code>NoSuchKey</Code>
<Message>The specified key does not exist.</Message>
</Error>
Whereas https://cdn.httparchive.org/static/icons/1C-Bitrix.png loads the image:
There was a problem hiding this comment.
Oh, you're right.
Let's use the full name WITH extension.
I'll fix the extension in tech data.
The icon property comes from a submitted detection rule, but then we do conversion to PNG.
At first, I thought I could adjust the icon names to your approach with technology names, but there are a number of icons that are reused by multiple technologies.
There was a problem hiding this comment.
Will update tomorrow to use the icon name with extension then 💯
There was a problem hiding this comment.
@max-ostapenko Is it normal that some techs still return an error? I guess some we just don't have an icon for?
For example this still gives an error:
https://cdn.httparchive.org/static/icons/Skilldo.png
{
"technology": "Skilldo",
"category": "CMS",
"description": "Skilldo is a content management system written in PHP and paired with a MySQL or MariaDB database.",
"icon": "Skilldo.png",
"origins": {
"mobile": 2027,
"desktop": 1573
}
}
There was a problem hiding this comment.
It's an issue on icons pipeline - icon exists, but wasn't picked up in the conversion step.
Need to adjust github action to keep all in sync - a couple of days for that.
tunetheweb
approved these changes
Mar 3, 2025
|
@sarahfossheim when I try this branch all the icons are broken. Does it need an update? |
|
It may be a backend issue: path in API inconsistent with location. |
|
Yes but the |
|
@tunetheweb I have a mixed set of results when I test:
So seemed to be a combo of the front-end like you mentioned (fixed now 🤞🏻) and the API like @max-ostapenko suggested. Doesn't work31 techs: (URL) 
Works30 techs: (URL) 
|
|
@tunetheweb also I see there's a conflict with the main branch meanwhile, will solve after our call today |
|
@sarahfossheim I resynced the icons. Do you still notice issues? |
|
@max-ostapenko icons work for me :) Also updated the dropdowns to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #1012
Name of the icons is now formatted differently, changing
%20. Example from the issues (Framer Sites) shows an icon in the table now.