Leaflet.DistortableImage icon indicating copy to clipboard operation
Leaflet.DistortableImage copied to clipboard

Review expensive HTTP operation in archive.html

Open segun-codes opened this issue 3 years ago • 5 comments

The code snippet below is extracted from /examples/archive.html, I am of the opinion that axios.get(...) shouldn't be invoked without validating that the user-supplied url argument meets certain criteria before attempting to perform the expensive over-the-network get operation albeit there's a failure handling mechanism in place.

I could improve the code by validating the url first, then wrap axios.get(...) codes in an if statement. For instance, if the url must always originate from https://archive.org, then the proposed validation will ensure this. Where validation fails, a small notification message is displayed to the user with the welcomeModal still in view.

@jywaren, what do you think? Can I give this a shot? Many thanks.

form.addEventListener('submit', (event) => {
      event.preventDefault();
      const url = input.value.replace('details', 'metadata');
      axios.get(url)
        .then((response) => {
          if (response.data.files && response.data.files.length != 0) {
            let imageCount = 0;
            response.data.files.forEach(file => {
              if (file.format === 'PNG' || file.format === 'JPEG'){
                let imageRow = document.createElement('div');
                let image = new Image(150, 150);
                let placeButton = document.createElement('a');

                placeButton.classList.add('btn', 'btn-sm', 'btn-outline-secondary', 'place-button');
                placeButton.innerHTML = 'Place on map';

                image.src = `${url.replace('metadata', 'download')}/${file.name}`;

segun-codes avatar Oct 19 '22 11:10 segun-codes

Hi @segun-codes, I think you can also try an async function.

liliyao2022 avatar Oct 20 '22 01:10 liliyao2022

Okay, thanks @liliyao2022.

@jywarren, @TildaDares still awaiting your comments on this issue. Thank you.

segun-codes avatar Oct 20 '22 12:10 segun-codes

Hi @segun-codes, can you tell us a bit more about how you plan to validate the url? Do you plan to use regex? Thank you!

TildaDares avatar Oct 20 '22 13:10 TildaDares

Hi @segun-codes, can you tell us a bit more about how you plan to validate the url? Do you plan to use regex? Thank you!

Hi @TildaDares,

  1. Plan is to use regex to ensure the url meets certain (as your prefer) criteria (e.g., valid url, originates from archive.org, image file extension should be .png | .jpeg | .jpg etc.).
  2. Then codes on line numbers 32 to 89 will simply be wrapped in an if block .
  3. If the url fails validation, then the user is informed via a small modal-based notification message.
  4. Note, at no point here will the welcomeModal become hidden.

What do you think? Your ideas are most welcome too @TildaDares @jywarren. Many thanks.

segun-codes avatar Oct 21 '22 10:10 segun-codes

Hi @TildaDares, I will proceed to fix and open a PR for this issue, many thanks.

segun-codes avatar Oct 22 '22 14:10 segun-codes