Use SVG/CSS alternative instead of "slick" carousel font
Slick carousel font still seems pretty gratuitous to @jdlrobson since all it does is provide 2 icons. He would prefer we drop it altogether. We could use an SVG as an alternative.
Have you considered using SVG/CSS and dropping this CSS as an alternative? :
/* Icons */
@font-face
{
font-family: 'slick';
font-weight: normal;
font-style: normal;
src: url(/static/css/fonts/slick.eot);
src: url(/static/css/fonts/slick.eot?#iefix) format('embedded-opentype'),
url(/static/css/fonts/slick.woff) format('woff'),
url(/static/css/fonts/slick.ttf) format('truetype'),
url(/static/css/fonts/slick.svg#slick) format('svg');
}
Originally posted by @jdlrobson in https://github.com/internetarchive/openlibrary/pull/3218
Hello, I would like to work on this. Can you please give me the link to how it looks now?
@cognitive137 I'm not sure why this got labelled a good first issue, but "Slick carousel" is this: https://github.com/kenwheeler/slick/ and you can see them in action on the Open Library home page. The font is used for the right and left arrows as well as a couple of other glyphs (I think 4 in all).
This is not a good first issue :)
@mekarpeles to expand on the above about this not being a good first issue, I'm not 100% sure if SVG would be preferable to the font. It's a hunch but not 100% certain. Whoever works on this should be familiar with how to measure performance impacts of changes.
It might help if we decide up front if we are planning to go with a CSS only solution or a SVG assets. If the latter we should provide those assets or suggest where they can be found or how they can derive - they don't exist yet. Would noun project suffice for example? Those SVGs will also need to be minified.
It's highly likely whoever works on this will be doing a lot of exploratory work and potentially realising that the font is actually preferable to an SVG asset (in which case we may not decide to merge any code and decline this task).
The pr https://github.com/internetarchive/openlibrary/pull/3218/files which aims at preloading custom fonts is not working as expected. The performance audit still shows slick.woff is not preloading. @mekarpeles @jdlrobson

@tabshaikh I can confirm. It looks like the preload tag is shown on https://openlibrary.org/books/OL6179353M/Slide_rule but not on the home page. I did see it on the home page but after refreshing it disappeared so I think this has something to do with caching.
Thanks for confirming @jdlrobson I will look into caching where it might be creating the problem
Hello is this issue still open?
I would love to work on this issue @mekarpeles please assign me this issue
@Spoorthy1423 if you'd like to try this (replacing the slick fonts with just the necessary used svgs) I think that would be welcome!
if this is still available, I would like to contribute, can this be assigned to me?
is it possible to work on this?
Certainly! Here is a professional work map for addressing the bug:
Step 1: Locate the Search Box Code
The search functionality likely resides in the frontend codebase. Here's how to locate it:
- Search for "search" in the
frontend/ortemplates/directories:grep -r "search" ./openlibrary/templates ./openlibrary/plugins - Likely files of interest:
templates/site_search.htmlstatic/js/autocomplete.jsor a similar file.
Step 2: Debug the Search Box
-
HTML Structure:
- Ensure proper semantic structure. For example:
<form action="/search" method="GET"> <input type="text" id="searchBox" name="q" placeholder="Search..." autocomplete="off"> <div id="searchDropdown"></div> </form>
- Ensure proper semantic structure. For example:
-
JavaScript Logic:
- Inspect event listeners for
onFocus,onBlur, oronInput. - Verify dropdown rendering and interaction.
- Inspect event listeners for
-
CSS Styling:
- Check for styling issues causing visual glitches (e.g., z-index or visibility).
Step 3: Apply Fixes
Case 1: Focus/Blur Issue
Update the event listeners to avoid unexpected blur or focus shifts:
const searchBox = document.getElementById('searchBox');
searchBox.addEventListener('focus', () => {
document.getElementById('searchDropdown').style.display = 'block';
});
searchBox.addEventListener('blur', () => {
setTimeout(() => {
document.getElementById('searchDropdown').style.display = 'none';
}, 200); // Delay to allow click on dropdown items
});
Case 2: Dropdown Suggestion Logic
Ensure dropdown suggestions are properly debounced:
let debounceTimeout;
searchBox.addEventListener('input', () => {
clearTimeout(debounceTimeout);
debounceTimeout = setTimeout(fetchSuggestions, 300);
});
function fetchSuggestions() {
const query = searchBox.value;
fetch(`/suggest?q=${query}`)
.then(response => response.json())
.then(data => {
renderDropdown(data);
});
}
function renderDropdown(data) {
const dropdown = document.getElementById('searchDropdown');
dropdown.innerHTML = data.map(item => `<div>${item}</div>`).join('');
}
Case 3: CSS Improvements
Ensure dropdown visibility is managed cleanly:
#searchDropdown {
position: absolute;
background-color: white;
border: 1px solid #ccc;
z-index: 1000;
display: none; /* Hidden by default */
}
Step 4: Test the Fix
- Manual Testing:
- Interact with the search box.
- Verify focus/blur behavior and dropdown suggestions.
- Automated Testing:
Add a test case to ensure the search box behaves correctly:
describe('Search Box', () => { it('should display dropdown on focus', () => { const searchBox = document.getElementById('searchBox'); searchBox.focus(); expect(document.getElementById('searchDropdown').style.display).toBe('block'); }); });
Step 5: Commit and Create a PR
- Commit the fix:
git checkout -b fix-search-box-3239 git add . git commit -m "Fix inconsistent search box behavior #3239" git push origin fix-search-box-3239 - Open a pull request, linking the issue, and provide a clear description of the fix.
Hey!! @mekarpeles sorry for not able to complete this issue on time, now i am ready to solve the issue, is this still open ??
@AciD-sEc thanks for taking a stab at breaking this down, give it a go!
It seems like this issue is stale, so unassigning.
@mekarpeles I opened a PR #10645 if you want to assign this issue to me