openlibrary icon indicating copy to clipboard operation
openlibrary copied to clipboard

Use SVG/CSS alternative instead of "slick" carousel font

Open mekarpeles opened this issue 5 years ago • 10 comments

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

mekarpeles avatar Mar 21 '20 23:03 mekarpeles

Hello, I would like to work on this. Can you please give me the link to how it looks now?

cognitive137 avatar Mar 23 '20 11:03 cognitive137

@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).

tfmorris avatar Mar 26 '20 02:03 tfmorris

This is not a good first issue :)

jdlrobson avatar Mar 26 '20 02:03 jdlrobson

@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).

jdlrobson avatar Mar 29 '20 16:03 jdlrobson

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 Screenshot from 2020-04-02 12-48-55 Screenshot from 2020-04-02 12-55-57

supercontracts avatar Apr 02 '20 07:04 supercontracts

@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.

jdlrobson avatar Apr 04 '20 18:04 jdlrobson

Thanks for confirming @jdlrobson I will look into caching where it might be creating the problem

supercontracts avatar Apr 04 '20 18:04 supercontracts

Hello is this issue still open?

khabdrick avatar Mar 02 '21 12:03 khabdrick

I would love to work on this issue @mekarpeles please assign me this issue

Spoorthy1423 avatar Jul 21 '24 16:07 Spoorthy1423

@Spoorthy1423 if you'd like to try this (replacing the slick fonts with just the necessary used svgs) I think that would be welcome!

mekarpeles avatar Aug 20 '24 13:08 mekarpeles

if this is still available, I would like to contribute, can this be assigned to me?

ChairBorn avatar Sep 13 '24 03:09 ChairBorn

is it possible to work on this?

psyren99 avatar Nov 12 '24 00:11 psyren99

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:

  1. Search for "search" in the frontend/ or templates/ directories:
    grep -r "search" ./openlibrary/templates ./openlibrary/plugins
    
  2. Likely files of interest:
    • templates/site_search.html
    • static/js/autocomplete.js or a similar file.

Step 2: Debug the Search Box

  1. 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>
      
  2. JavaScript Logic:

    • Inspect event listeners for onFocus, onBlur, or onInput.
    • Verify dropdown rendering and interaction.
  3. 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

  1. Manual Testing:
    • Interact with the search box.
    • Verify focus/blur behavior and dropdown suggestions.
  2. 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

  1. 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
    
  2. Open a pull request, linking the issue, and provide a clear description of the fix.

AciD-sEc avatar Dec 13 '24 13:12 AciD-sEc

Hey!! @mekarpeles sorry for not able to complete this issue on time, now i am ready to solve the issue, is this still open ??

Spoorthy1423 avatar Dec 13 '24 13:12 Spoorthy1423

@AciD-sEc thanks for taking a stab at breaking this down, give it a go!

mekarpeles avatar Dec 30 '24 20:12 mekarpeles

It seems like this issue is stale, so unassigning.

mekarpeles avatar Jan 14 '25 19:01 mekarpeles

@mekarpeles I opened a PR #10645 if you want to assign this issue to me

jrutheiser avatar Mar 31 '25 19:03 jrutheiser