Download picture from gallery

[rawbertp]

Allow download of pictures from gallery.

Should fix #7 .

[andi34]

Could we make this an option in admin panel?

[rawbertp]

@sualko Please review the img refactoring - it works but I am not sure if this is the correct approach!

@andi34 - sure why not ;)

[andi34]

You'll take care of it? Or should I look at it tomorrow?

[rawbertp]

I'm on it. However, I was just thinking that it wouldn't make sense to allow QR but no download and vice versa. I would combine it: if DL is enabled, both will be shown QR and DL button. OK with that?

[rawbertp]

I have now implemented it as separate option. Still, I don't think this makes sense, but it can be easily changed once there is an agreement. Or left as is... ;-)

[andi34]

I'd make it an own option independent of QR, gives people all possibilities to fit personal needs.
Maybe someone likes none of it, for me only the DL button would be used (no one i know really uses QR codes).

[andi34]

Just tested, working fine :) thanks a lot!!!

[andi34]

Only one thing we could change is to hide the button if we're accessing the page from our Pi (hide on access from IP of the Pi, localhost and 127.0.0.1).

[andi34]

maybe you could cherry-pick and push this one here too andi34@366ea66

[rawbertp]

@andi34 I added a check for SERVER_ADDR != REMOTE_ADDR and added the stopPropagation call. The other change you mentioned will get merged anyway, so I didn't cherry-pick it.

[sualko]

Thanks for your contribution, but I think there are still some details to fix. See my comments and drop your two cents.

[sualko]

window.location would be better than location (because it makes clear that location is no local variable), but the html5 download attribute would be even better. If you create a new a element at this position with the corresponding attributes a emulated click event on that element should work.

[rawbertp]

If I get you right, you suggest to replace <button class="pswp__button pswp__button--download" ... in pwswp.template.php (i.e. the current approach that applies to all the other buttons there) with an <a href="<js-injected-url>" download="" ... ?

[sualko]

No, I meant something like this (untested):

const linkElement = $('<a>');
linkElement.attr({
   href: 'URL_TO_IMAGE',
   download: 'FILENAME',
});
linkElement.click();

This should ensure that the browser shows the download prompt instead of open the image.

[andi34]

Also check if download attribute is supported by the browser?

function doesAnchorSupportDownload() {
    return "download" in document.createElement("a")
}

Currently I am checking it this way

        if (doesAnchorSupportDownload()) {
            location = 'api/download.php?image=' + img;
        } else {
            alert('Unsupported on your Browser');
        }

[rawbertp]

Sorry, I’m afraid I don’t get it. „On click“ the element gets manipulated and then .click() gets called? What’s the benefit?

I guess I will leave this PR open and leave it up to you (or s.o. else with web developer skills) to properly implement this feature. 😌

[rawbertp]

@sualko Out of curiosity I still tried to fix this according to your suggestions, see: 7d98647 . It works but might be complete bullsh... Looking forward to your feedback. ;-)

[sualko]

That was smart. Even better than my solution. I made only a couple of minor changes, otherwise a great pr. Thanks a lot.

@andi34 there is no need to check for support. If the browser doesn't know the attribute it get's ignored.

[andi34]

@sualko but wouldn't it be better to tell the people instead silent not working?

[rawbertp]

@sualko Thanks, but that was a bit too quick now, I guess... You merged just the first but not the latest commit. I suggest to squash these on my branch and then merge the PR?

[andi34]

@rawbertp He did already ;) c4ace13

[rawbertp]

Ah ok i see.. Okay but there might be one minor issue left with the button. I haven't looked into it yet as I first wanted to wait if this gets accepted at all. IIRC the button was not correctly aligned with the others - I will have to have a look...

[andi34]

@rawbertp haven't checked yet, but he also added the line hight here:
c4ace13#diff-28f3b1da8414261c1172ba056d85e851R13

[rawbertp]

Alright, looks good (also on the iPad). :)

[sualko]

He did already

Yes, he did 😂

but wouldn't it be better to tell the people instead silent not working?

That's no problem. The browser will still show the picture.