Add Search feature to Browse and Download Pages #39
Conversation
|
I would not recommend to do that in javascript only. |
|
Thank you @44yu5h ; I'll review it in a few days. |
rgaudin
left a comment
rgaudin
left a comment
There was a problem hiding this comment.
Thank you for the PR ; it works well.
Beside inline comments I see a couple of things to fix:
- focus color on text field is missing. Just set it to whatever and we'll choose one but on my computer, it uses the default bright blue which is harsh.
- there is no text filter on mobile mode while that's probably where it would be most useful. You have to duplicate the field, as we did for the other filters.
| order_dir = 'desc'; | ||
| only_lang = ''; | ||
| only_category = ''; | ||
| only_search = ''; |
There was a problem hiding this comment.
Rename to only_pattern which is more clear
| <label | ||
| for="search-bar" | ||
| class="hidden lg:inline-block kiwix-{{page }}-filter-label text-[0.9em] me-[1em] dm-sans" | ||
| >Search</label> |
There was a problem hiding this comment.
Find as label seems more appropriate (albeit not perfect). Search word sort of implies it will look at content while this is just a free-text filter on title/desc.
@siemsie any suggestion on the label ?
| <input | ||
| id="search-bar" | ||
| type="text" | ||
| placeholder="Search..." |
There was a problem hiding this comment.
| placeholder="Search..." | |
| placeholder="Filter on titles and descriptions…" |
| id="search-bar" | ||
| type="text" | ||
| placeholder="Search..." | ||
| class="select select-bordered select-sm max-w-xs dm-sans h-[2.25rem]" |
There was a problem hiding this comment.
We cannot have dm-sans here as this is free text and users can input any char while dmsans is latin only.
| visible = false; | ||
| if (visible && this.only_search.trim() !== '') { | ||
| const nameAttr = elems[i].getAttribute('data-name').toLowerCase(); | ||
| const descAttr = elems[i].getAttribute('data-desc') ? elems[i].getAttribute('data-desc').toLowerCase() : ''; |
There was a problem hiding this comment.
Can you retrieve the description from the DOM below instead? This would force duplicating the descriptions
| data-name="{{ package.title }}" | ||
| data-size="{% if package.download and package.download.size %}{{ package.download.size }}{% else %}0{% endif %}"> | ||
| data-size="{% if package.download and package.download.size %}{{ package.download.size }}{% else %}0{% endif %}" | ||
| data-desc="{{ package.description|default('')|escape }}" |
| data-langs="{% if package.langs %}{% for lang in package.langs %}{{ lang.alpha_3 }} {% endfor %}{% endif %}" | ||
| data-name="{{ package.title }}" | ||
| data-size="{% if package.download and package.download.size %}{{ package.download.size }}{% else %}0{% endif %}" | ||
| data-desc="{{ package.description|default('')|escape }}" |
|
@siemsie test |
3 participants
Feat: Search
Alt: UI
Alt: Searching in description
Closes #35