Skip to content
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

Don't block image search page if bing is slow #563

Open
jzohrab opened this issue Jan 8, 2025 · 5 comments
Open

Don't block image search page if bing is slow #563

jzohrab opened this issue Jan 8, 2025 · 5 comments
Labels
code-improvement Make code/structure better good first issue Good for newcomers tweak Adjustment to the code -- not a new feature, just minor mods/tweaks

Comments

@jzohrab
Copy link
Collaborator

jzohrab commented Jan 8, 2025

Currently, the image search makes a request to bing and waits for a response before showing the image list. That can look slow or weird if bing is having trouble.

Instead the page should load instantly, and then make a separate ajax request to get the bing image search content.

@jzohrab jzohrab added code-improvement Make code/structure better tweak Adjustment to the code -- not a new feature, just minor mods/tweaks labels Jan 8, 2025
@jzohrab jzohrab added this to Lute-v3 Jan 8, 2025
@jzohrab jzohrab added the good first issue Good for newcomers label Jan 8, 2025
@jzohrab
Copy link
Collaborator Author

jzohrab commented Jan 8, 2025

"good first issue" as I think this is relatively self-contained.

@parradam
Copy link

Hey @jzohrab ! I'm obviously very new to the codebase, but thought I could have a crack at this one.

If I understand you correctly, the UI thread is blocked while waiting for the image search response from Bing.

I would propose something like the code below. Essentially showing a loading message and an then making an async fetch for the Bing content, then displaying it. Seems to work as intended.

Please let me know if this is what you were after - happy to discuss further and raise a PR!

PS: I see that we could probably cache the images too, maybe that's a further improvement!

class ImageLookupButton extends GeneralLookupButton {
  constructor() {

    // Parents are in the tagify-managed #parentslist input box.
    let _get_parent_tag_values = function() {
      const pdata = LookupButton.TERM_FORM_CONTAINER.querySelector("#parentslist").value
      if ((pdata ?? '') == '')
        return [];
      return JSON.parse(pdata).map(e => e.value);
    };

    let handler = async function(iframe) {
      const text = LookupButton.TERM_FORM_CONTAINER.querySelector("#text").value;
      const lang_id = LookupButton.LANG_ID;
      if (lang_id == null || lang_id == '' || parseInt(lang_id) == 0 || text == null || text == '') {
        alert('Please select a language and enter the term.');
        return;
      }
      let use_text = text;

      // If there is a single parent, use that as the basis of the lookup.
      const parents = _get_parent_tag_values();
      if (parents.length == 1)
        use_text = parents[0];

      const raw_bing_url = 'https://www.bing.com/images/search?q=[LUTE]&form=HDRSC2&first=1&tsc=ImageHoverTitle';
      const binghash = raw_bing_url.replace('https://www.bing.com/images/search?', '');
      const url = `/bing/search/${LookupButton.LANG_ID}/${encodeURIComponent(use_text)}/${encodeURIComponent(binghash)}`;

      // Changed code
      iframe.srcdoc = '<p>Loading images from Bing...</p>';

      try {
        const response = await fetch(url);
        if (!response.ok) {
          throw new Error(`Error: ${response.statusText}`);
        }

        const data = await response.text()
        iframe.srcdoc = data;
      }
      catch(e) {
        console.log(e);
        iframe.srcdoc = '<p>Images could not be loaded from Bing.</p>';
      }
    };  // end handler

    super("dict-image-btn", null, "Lookup images", "dict-image-btn", handler);
  }
}

@jzohrab
Copy link
Collaborator Author

jzohrab commented Jan 15, 2025

Hi @parradam , thanks very much!

Sorry for the bad/incomplete spec :-) I was thinking of something different: immediately opening the /bing/search/ page with the query strings, and having that page kick off the JS call. The only reason for this is that the links to upload an image, or click-here-then-past, would be immediately visible, and their JS handlers would be available, even if the bing call fails.

The template that renders the image response is at imagesearch/index.html -- do you think you could try having that page make the js call instead? If it's crazy messy, then yours is a good first step.

Cheers!

@parradam
Copy link

Thanks @jzohrab !

No problem, it's been useful for me to understand how the templates, JS, and backend fit together! :)

I think I have a working solution using the template (confirmed using the DOMContentLoaded breakpoint as sometimes it's hard to see!). I'll do a sanity check tomorrow and then raise a PR!

Thanks!

@jzohrab
Copy link
Collaborator Author

jzohrab commented Jan 15, 2025

Very thankful!

To test it: turn off your wifi :-)

cheers, will be very nice to have this. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-improvement Make code/structure better good first issue Good for newcomers tweak Adjustment to the code -- not a new feature, just minor mods/tweaks
Projects
Status: No status
Development

No branches or pull requests

2 participants