-
-
Notifications
You must be signed in to change notification settings - Fork 45
Use custom multi select dropdowns for the tech selector #1043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
from meeting:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Will give it a full review when you're finished but for now spotted one this.
option.id = `${this.element.dataset.id}-${row}`; | ||
const logo = document.createElement('img'); | ||
logo.setAttribute('alt', ''); | ||
logo.setAttribute('src', `https://cdn.httparchive.org/static/icons/${row}.png`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes the icon is the technology name, but that's not always the case.
For example:
- "Yandex SmartCaptcha" - "yandex_smartcaptcha.png"
It also doesn't work when a technology reuses an icon. For example:
"Yandex.Cloud": {
"cats": [
62
],
"description": "Yandex.Cloud is a public cloud platform where companies can create and develop projects using Yandex's scalable computing power, advanced technologies, and infrastructure.",
"icon": "Yandex.Cloud.svg",
"pricing": [
"payg"
],
"saas": false,
"website": "https://cloud.yandex.com/en/"
},
"Yandex.Cloud CDN": {
"cats": [
31
],
"description": "Yandex.Cloud CDN helps you streamline static content delivery for your web service.",
"dom": "[href*='storage.yandexcloud.net'], [src*='storage.yandexcloud.net']",
"icon": "Yandex.Cloud.svg",
"implies": "Yandex.Cloud",
"website": "https://cloud.yandex.com/en/services/cdn"
},
Can we pull in the actual icon name from the API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but then we need to drop the ?onlyname
from the request to the technologies endpoint, and we added it a while back to speed up the API response 😅
So options are:
- Fetch the icons for the dropdowns using
/v1/technologies
, have a slower API response - Keep the icons in the dropdowns based on the names (
/v1/technologies?onlyname
), have some icons not show up - Remove the icon from the dropdowns
@max-ostapenko it seems that your open PR #1048 will speed up the technologies endpoint, do you think it will be ok to use it again without the onlyname
for the dropdowns?
Co-authored-by: Barry Pollard <[email protected]>
Co-authored-by: Barry Pollard <[email protected]>
…nto cwvtech-dropdown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and we'll fix the icons in a future PR
Changed
Related to
Screenshots
Input field, collapsed

Input field, open, nothing searched
Searching for a tech
Tech selected
Multiple tech selected
Multiple tech selected, not filtered
10 techs selected, unable to pick more