Added minimize button to popup

[huggla]

This is my attempt to add a minimize button to popup (fix for #1945). It works, but I would appreciate if someone with more experience could make the code less ugly.

[huggla]

[Grammostola]

I think it's a neat feature. Running the default map config and infoclicking an Origo city makes the overlay not show its contents and move to the right?

As for pretty you could formulate it like the close button's event, meaning no event handler declaration in the html string:

          <button id="o-minimize-button" class="small round margin-top-smaller margin-bottom-auto margin-right-small icon-smallest grey-lightest no-shrink" aria-label="Minimera">
            <span class="icon ">_</span>
          </button>

then a function where the other functions reside:

function minimizePopup() {
  const oidentify = document.getElementById('o-identify');
  const opopup = document.getElementById('o-popup');
  oidentify.style.display = oidentify.style.display === 'none' ? 'block' : 'none';
  opopup.style.width = oidentify.style.display === 'none' ? 'auto' : null;
}

then the event handler by the close one:

  function bindUIActions() {
    const closeel = document.querySelector('#o-popup .o-popup #o-close-button');
    closeel.addEventListener('click', (evt) => {
      closePopupInternal(closeCb);
      evt.preventDefault();
    });

    const minel = document.querySelector('#o-popup .o-popup #o-minimize-button');
    minel.addEventListener('click', (evt) => {
      minimizePopup();
      evt.preventDefault();
    });
  }

Something along those lines is what I would try anyways.

[huggla]

Thanks for your input. I tried your code and it seems to work as intended, but I don't understand the part below. Could you please explain the syntax? Is (evt) => {} shorthand for function(evt) {}? What is the purpose of evt.preventDefault()? Is it strictly needed? Why not simply write minel.addEventListener('click', minimizePopup);? minel.addEventListener('click', (evt) => { minimizePopup(); evt.preventDefault(); });

…

________________________________________ Från: David ***@***.***> Skickat: den 16 februari 2024 08:22 Till: origo-map/origo Kopia: Henrik Uggla; Author Ämne: Re: [origo-map/origo] Added minimize button to popup (PR #1949) I think it's a neat feature. Running the default map config and infoclicking an Origo city makes the overlay not show its contents and move to the right? As for pretty you could formulate it like the close button's event, meaning no event handler declaration in the html string: <button id="o-minimize-button" class="small round margin-top-smaller margin-bottom-auto margin-right-small icon-smallest grey-lightest no-shrink" aria-label="Minimera"> <span class="icon ">_</span> </button> then a function where the other functions reside: function minimizePopup() { const oidentify = document.getElementById('o-identify'); const opopup = document.getElementById('o-popup'); oidentify.style.display = oidentify.style.display === 'none' ? 'block' : 'none'; opopup.style.width = oidentify.style.display === 'none' ? 'auto' : null; } then the event handler by the close one: function bindUIActions() { const closeel = document.querySelector('#o-popup .o-popup #o-close-button'); closeel.addEventListener('click', (evt) => { closePopupInternal(closeCb); evt.preventDefault(); }); const minel = document.querySelector('#o-popup .o-popup #o-minimize-button'); minel.addEventListener('click', (evt) => { minimizePopup(); evt.preventDefault(); }); } Something along those lines is what I would try anyways. — Reply to this email directly, view it on GitHub<#1949 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABIEJV7SMB5V67RAIIM6Q73YT4CJVAVCNFSM6AAAAABCFDVMECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBXHA3TINJWHA>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[huggla]

Also, what are the pros, in this case, to adding event listeners at runtime vs. having them in the static html? (I'm trying to learn)

[Grammostola]

s (evt) => {} shorthand for function(evt) {}?

yes

What is the purpose of evt.preventDefault()? Is it strictly needed?

Based on my non-complete understanding the preventDefault method prevents the default browser method. What's the default browser method of a button click? There doesn't seem to be one. Removing it doesn't have an obvious effect so I would probably suggest that. (I merely echoed the existing button's setup in the example above. You are right to question what doesn't seem right)

Why not simply write minel.addEventListener('click', minimizePopup);?

No reason but to keep things consistent. That's not a hard requirement though so I'd call it optional. And your way still has access to the event object (even if we don't appear to need it)

Also, what are the pros, in this case, to adding event listeners at runtime vs. having them in the static html?

Writing code as a string is unpleasant and hard to maintain for one

As for the functionality, when I test it with the default Origo map config the minimize click makes the popup go sideways.

[huggla]

Thanks for your clarification and input. I can confirm that the button doesn't work in the default configuration and I'll try to fix it.

[huggla]

I think it should be fixed now.

[Grammostola]

Lgtm (the preventDefault() can both be removed in a separate pr some other time)