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

Improve Image gallery #567

Closed
wants to merge 6 commits into from
Closed

Improve Image gallery #567

wants to merge 6 commits into from

Conversation

talha131
Copy link
Member

@talha131 talha131 commented Jan 30, 2020

@talha131
Copy link
Member Author

@iranzo can you please give an example instagram_id that has a single image in it? I do not use Instagram at all so I can't do it myself.

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

@iranzo can you please give an example instagram_id that has a single image in it? I do not use Instagram at all so I can't do it myself.

B7yh4IdItNd ( https://www.instagram.com/p/B7yh4IdItNd/ )

Until some releases ago ( I didn't knew), it just allowed single photos, afaik the max now is 10

@talha131
Copy link
Member Author

About photoswipe: 'https://photoswipe.com/documentation/faq.html#image-size' Requires to know 'in advance' image dimensions, so additional preprocessing is needed (just FYI)

Originally posted by @iranzo in #385 (comment)

Thank you for pointing it out.

I am looking through your code. It first fetches JSON from Instagram.

http GET "https://www.instagram.com/p/B3ePmL7gd1P/?__a=1"

A relevant JSON snippet is this,

              "display_url": "https://instagram.fisb5-1.fna.fbcdn.net/v/t51.2885-15/e35/70773817_445528152735758_7592776886780077817_n.jpg?_nc_ht=instagram.fisb5-1.fna.fbcdn.net&_nc_cat=102&_nc_ohc=c75zP7rxwj0AX_T7LqS&oh=bdbd853a5bf231aea71cb19f63243dfd&oe=5EBF4F25",
              "display_resources": [
                {
                  "src": "https://instagram.fisb5-1.fna.fbcdn.net/v/t51.2885-15/sh0.08/e35/s640x640/70773817_445528152735758_7592776886780077817_n.jpg?_nc_ht=instagram.fisb5-1.fna.fbcdn.net&_nc_cat=102&_nc_ohc=c75zP7rxwj0AX_T7LqS&oh=790f184b61461d9b6b583f3690f00649&oe=5EC6A3C0",
                  "config_width": 640,
                  "config_height": 582
                },

It seems Instagram returns the width and height of the image. We can leverage it.

But I plan to use this PR to improve all other images in the Elegant.

Can you investigate if Pelican's photos plugin, that we already support, returns image dimension to the theme?

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

My idea was not to focus this just for instragram (the javascript code I wrote, does the automated part), but gallery allows to prefill information with the json (mentioned on the other thread), so galleria.io can be used for videos, twitter static images (pbs.XXX urls) etc (that's why the original issue was updated to be not just 'flickr' or 'instagram' to gallery

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

I'll check on photos plugin we do use right now (which it should have access as it requires to have the images locally to be transformed, captioned, etc before pushing to the generated site)

@talha131
Copy link
Member Author

My idea was not to focus this just for instragram (the javascript code I wrote, does the automated part), but gallery allows to prefill information with the json (mentioned on the other thread), so galleria.io can be used for videos, twitter static images (pbs.XXX urls) etc (that's why the original issue was updated to be not just 'flickr' or 'instagram' to gallery

We both are on the same page here. I too want this code to work for as many platforms as possible.

My plan is to make it work with,

  1. Photos plugin
  2. Liquid tags plugin
  3. Instagram

As for Flickr and Twitter, it would definitely require changes in the JS code because each service will returns its own JSON. But the idea will remain the same.

Gallaeria.io can be used for videos, but its dependecny on jquery is a blocker.

Photoswipe is limited to images but has better keyboard and gesture controls.

Let me know if I am missing something here. It is necessary that we both are clear on the end goal and plan.

@talha131
Copy link
Member Author

talha131 commented Jan 30, 2020

@iranzo

Photos plugin does not send image size to the theme. Liquid tag plugin lets user define height and width, but if the user does not define it then theme cannot know the image size.

Both these plugins can be modified to send image dimensions to the theme. But we need dimension only if we use PhotoSwipe framework.

If we use light gallery then we do not need to worry about image size at all.

Secondly, if we compare the getting started guide of both these libraries, then Light Gallery is far easier to integrate.

  1. https://sachinchoolur.github.io/lightgallery.js/docs/#installation
  2. https://photoswipe.com/documentation/getting-started.html

Development has stopped on Light Gallery. But, unless we face a showstopper bug, we can still use it. Light gallery also supports videos.

What do you think?

If we decide to use PhotoSwipe, then we can ask user to add image dimensions to the filename, like holiday--512--1024.png. JS can extract dimensions and pass it to the PhotoSwipe.

@talha131
Copy link
Member Author

Here are few old threads that compare these two libraries

  1. https://www.reddit.com/r/webdev/comments/3xo4e5/photoswipe_vs_lightgallery/
  2. https://colormelon.com/best-javascript-popup-gallery/

These are outdates because light gallery does not depend on jquery.

@talha131
Copy link
Member Author

I tried both these libraries on my mobile. Sadly, light gallery performed less than impressively.

Transitions and gestures in PhotoSwipe worked flawlessly. Due to this, I vote for PhotoSwipe.

I recommend you to open demos of both galleries on your mobile devices and test them. Then let me know which one you like.

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

Photoswipe for me... zoom with two fingers is very intuitive

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

For me, having to get image size provided by user on the data file to load is a inconvenient, I would prefer adding code in JS to get the size and prepulate as needed (more overhead, but for end user as easy as point to image, title, etc and that's all)

According to their guide, we should do that on step3, for example, in case we don't have w and h for each image.

Photos plugin adds features like resizing, 'copyright', etc

As I like the gallery itself we can work here to get this gallery for the new features and later see how to rework photo plugin integration to combine both.

As the goal in the templates as of this, is a 'gallery' I would prefer to have the 'slide' approach listed which shows miniatues of the pics so that you can click, similar to what there's at 'https://photoswipe.com/' unless we can get (as a less preferred alternative) autoplay feature like galleria.io has.

Feedback?

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

Pelican theme using PhotoSwipe (probably we can get data from there):

https://github.com/abjorck/pelican-mg-cleaner/blob/master/templates/gallery.html

@iranzo
Copy link
Member

iranzo commented Jan 30, 2020

            var slides = [];
            {% for name, photo, thumb, exif, caption, size in article.photo_gallery %}
                slides.push({ src: '{{photo}}', msrc: '{{thumb}}', w: {{size[0]}}, h: {{size[1]}}, title: '{{ caption }}' });
            {% endfor %}

@talha131
Copy link
Member Author

For me, having to get image size provided by user on the data file to load is a inconvenient, I would prefer adding code in JS to get the size and prepulate as needed (more overhead, but for end user as easy as point to image, title, etc and that's all)

I agree.

As I like the gallery itself we can work here to get this gallery for the new features and later see how to rework photo plugin integration to combine both.

Yeah, I agree. End goal is to support gallery and individual images. If Photos plugin can help us then we will add it otherwise we won't.

At the moment, I am open to writing a new plugin to add support. But let's first see how PhotoSwipe works.

As the goal in the templates as of this, is a 'gallery' I would prefer to have the 'slide' approach listed which shows miniatues of the pics so that you can click, similar to what there's at 'https://photoswipe.com/' unless we can get (as a less preferred alternative) autoplay feature like galleria.io has.

I agree.

Pelican theme using PhotoSwipe (probably we can get data from there):

Can you give this theme a try? I don't think Photo plugin provides size, which this theme uses. They probably have a fork of Photos plugin.


Correct me if I am wrong, your original patch would add gallery from instagram only at the bottom the article? This means if user wants to add a gallery in the middle of the article then it won't work.

@talha131
Copy link
Member Author

talha131 commented Jan 30, 2020

@iranzo Kindly comment on the following.

Let's forget about plugins for a moment. We know we want an image gallery that can work for any number of image including single image.

All we need is this snippet in a markdown file.

  <div class="my-gallery" itemscope itemtype="http://schema.org/ImageGallery">

        <figure itemprop="associatedMedia" itemscope itemtype="http://schema.org/ImageObject">
          <a href="https://farm3.staticflickr.com/2567/5697107145_a4c2eaa0cd_o.jpg" itemprop="contentUrl" data-size="1024x1024">

<img src="https://farm3.staticflickr.com/2567/5697107145_3c27ff3cd1_m.jpg" itemprop="thumbnail" alt="Image description"  />
    </a>
    </figure>
</div>

With the help of PhotoSwipe and JS, we can turn it into a gallery. You can view a live example here

https://codepen.io/talha131/pen/abzgJea

It also shows we can have the above snippet anywhere in the article. It will automatically turn into a gallery. I mean gallery will NOT always be present at the bottom of the article.

Now the only question is how do we get this information

  1. Original image, https://farm3.staticflickr.com/2567/5697107145_a4c2eaa0cd_o.jpg
  2. Size of original image,
  3. Thumbnail, https://farm3.staticflickr.com/2567/5697107145_3c27ff3cd1_m.jpg

Answer is Photos plugin. It is the only plugin that generates thumbnails and can process size.

This leaves us with these questions

  1. Can we get image size using Photos plugin?
  2. Can we use Photos plugin for solitary images in an article? For example, there is a single image under heading "ABC". Then there is another image under heading "DEF". We want both these images to work with PhotoSwipe.

If Photos cannot do it then we can fork it and add these features.

Finally, if it is a gallery of images then it's thumbnails should be presented in an elegant manner.

@iranzo
Copy link
Member

iranzo commented Jan 31, 2020

Can you give this theme a try? I don't think Photo plugin provides size, which this theme uses. They probably have a fork of Photos plugin.

Yup, that was my idea

Correct me if I am wrong, your original patch would add gallery from instagram only at the bottom the article? This means if user wants to add a gallery in the middle of the article then it won't work.

Yes, that's right, similar to the actual gallery available using the 'photos' plugin.

@iranzo
Copy link
Member

iranzo commented Jan 31, 2020

All we need is this snippet in a markdown file.

  <div class="my-gallery" itemscope itemtype="http://schema.org/ImageGallery">

        <figure itemprop="associatedMedia" itemscope itemtype="http://schema.org/ImageObject">
          <a href="https://farm3.staticflickr.com/2567/5697107145_a4c2eaa0cd_o.jpg" itemprop="contentUrl" data-size="1024x1024">

<img src="https://farm3.staticflickr.com/2567/5697107145_3c27ff3cd1_m.jpg" itemprop="thumbnail" alt="Image description"  />
    </a>
    </figure>
</div>

With the help of PhotoSwipe and JS, we can turn it into a gallery. You can view a live example here

https://codepen.io/talha131/pen/abzgJea

It also shows we can have the above snippet anywhere in the article. It will automatically turn into a gallery. I mean gallery will NOT always be present at the bottom of the article.

That's good when writing longer articles willing to ilustrate on something (f.e. tutorials)

Now the only question is how do we get this information

  1. Original image, https://farm3.staticflickr.com/2567/5697107145_a4c2eaa0cd_o.jpg
  2. Size of original image,
  3. Thumbnail, https://farm3.staticflickr.com/2567/5697107145_3c27ff3cd1_m.jpg

Can we force thumbnail to be equal to original image? or must be reduced? Have you tested putting the same file on both?

If so, I'll go for reusing image as thumbnail (so it's one resource less to load/generate) and then js for the image size (I'll check on that)

Answer is Photos plugin. It is the only plugin that generates thumbnails and can process size.

This leaves us with these questions

  1. Can we get image size using Photos plugin?

We can probably get that info from the data in the image or run a commandline tool or python for getting it.

  1. Can we use Photos plugin for solitary images in an article? For example, there is a single image under heading "ABC". Then there is another image under heading "DEF". We want both these images to work with PhotoSwipe.

Photos afaik just processes the images and does the transformation (embed text, etc) and copies to final location via available article metadata variables.

If Photos cannot do it then we can fork it and add these features.

I would make both, a fork in the meantime and PR to original so that it gets mantained

Finally, if it is a gallery of images then it's thumbnails should be presented in an elegant manner.

+1

@talha131
Copy link
Member Author

I would make both, a fork in the meantime and PR to original so that it gets mantained

@iranzo we need to do it correctly. The issue is, we know gallery is not something that must always be appended to the bottom. User should be able to place it anywhere in his article, top, middle, bottom. He can also have more than one galleries in an article.

What we need is to create a new plugin that will use Photos code. What this new plugin should do?

It defines a new syntax that user can use to include gallery anywhere in his article.

Let's say the syntax for Instagram gallery is $$instagram_id:<id>$$. Now user's markdown is

## Hello

These are the pictures from the concert.

$$instagram_id:1234$$

These are from the last year.

$$instagram_id:9876$$

Now the plugin will pick this Instagram gallery, and convert into to

<div class="my-gallery" itemscope itemtype="http://schema.org/ImageGallery">

    <figure itemprop="associatedMedia" itemscope itemtype="http://schema.org/ImageObject">
          <a href="https://farm3.staticflickr.com/2567/5697107145_a4c2eaa0cd_o.jpg" itemprop="contentUrl" data-size="1024x1024">

<img src="https://farm3.staticflickr.com/2567/5697107145_3c27ff3cd1_m.jpg" itemprop="thumbnail" alt="Image description"  />
    </a>
    </figure>
</div>

So you see, user has included two galleries in between the article.

User can use similar syntax for galleries.

## Example heading

$$gallery
{
        "caption": "Local image",
        "image": "{static}/images/example.png",
  },
{
        "caption": "KubeVirt booth",
        "image": "https://pbs.twimg.com/media/EPCJwpuXsAAGpHm?format=jpg",
  },
    {
        "description": "KubeVirt booth",
        "image": "https://pbs.twimg.com/media/EPHwd4zWAAAg8Y-?format=jpg",
    },
$$

This is just an example syntax.

The plugin, will take up this and change into the div I mentioned above.

It will also create thumbnails from the images and report image sizes. This is where it will use selected code from the Photos plugin.

@talha131
Copy link
Member Author

talha131 commented Feb 1, 2020

@iranzo What do you think of my proposal? Do you think you would be able to create such a plugin?

If it is not possible in the near future, then an alternative solution is this.

For gallery, user will have to manually write the <div>, and create and maintain thumbnails. We will not be using an plugin for it.

For Instagram gallery, user can write <div class="elegant-instagram" data-galllery-id="abc-123"></div>, in this markdown. Your JS script will then do its job. It will fetch the images and replace the div element with the gallery code.

This is a quick solution, though not as powerful as having a proper plugin.

Let me know your thoughts. Which solution you like, and which part of the code you would like to work on?

@iranzo
Copy link
Member

iranzo commented Feb 1, 2020 via email

@talha131
Copy link
Member Author

talha131 commented Feb 2, 2020

@iranzo check this out please

https://next.elegant.oncrashreboot.com/photoswipe-gallery-using-raw-html

Our next step will be done on top of it.

@iranzo
Copy link
Member

iranzo commented Feb 4, 2020

Done in documentation/content/2020-01-27-instagram.md

@talha131
Copy link
Member Author

talha131 commented Feb 4, 2020

My result

You kick start the fetch data from Instagram. And immediately afterwards set the inner html. Fetch takes time. Therefore, when you are setting the inner html, at that time content is undefined.

I just changed to the code to set inner html after we have got the images and html is ready.

Now,

  1. Please do not use jQuery. We are moving away from it. That;s the reason for all this effort in integrating with PhotoSwipe.
  2. Your article should have a simple div.
## Heading 1

<div class="photoswipe-instagram" data-instagram-id="example123"></div>

## Heading 2

<div class="photoswipe-instagram" data-instagram-id="example456"></div>

The HTML that you will generate has to be exactly like the one described here. It means resultant html will have no <div class="photoswipe-instagram" data-instagram-id="example456"></div>.

Therefore instead of setting innerHTML, just replace the element.

@iranzo
Copy link
Member

iranzo commented Feb 4, 2020

Thanks,
I'm not an expert in javascript nor Jquery, so that's the reason to first try to get something working and then try to get pieces replaced.

I'll continue from the changes you made, thanks a lot!

@talha131
Copy link
Member Author

talha131 commented Feb 4, 2020 via email

@talha131
Copy link
Member Author

talha131 commented Feb 5, 2020

@iranzo I am going to change the css class to elegant-gallery from photoswipe-gallery.

Your class should be elegant-instagram.

Let me know if you have any input on this. I will probably release this change today.

@talha131
Copy link
Member Author

talha131 commented Feb 5, 2020

BTW Great job! We are very close!

@iranzo iranzo force-pushed the feat-image-gallery branch from 99cc0c8 to c001766 Compare February 5, 2020 14:36
@iranzo
Copy link
Member

iranzo commented Feb 5, 2020

I was cleaning the code, article is left as it was, and code is now in base, my plan would be to move it back to article and keep testing

@iranzo
Copy link
Member

iranzo commented Feb 5, 2020

Updated also the example article, moved into the documentation one, etc

@iranzo iranzo force-pushed the feat-image-gallery branch 6 times, most recently from 0c446ff to 498dcff Compare February 5, 2020 15:50
@iranzo
Copy link
Member

iranzo commented Feb 5, 2020

Need to review as there seems to be a problem now with the move from base to article

@talha131
Copy link
Member Author

talha131 commented Feb 5, 2020

@iranzo after you set inner html, try call this method on your div. initPhotoSwipeFromDOM(".photoswipe-gallery");

When your script runs, by that initPhotoSwipeFromDOM has already run. So your div never get passed to initPhotoSwipeFromDOM.

That's why you need to trigger it manually.

@iranzo iranzo force-pushed the feat-image-gallery branch from 498dcff to af391bf Compare February 5, 2020 18:39
@iranzo
Copy link
Member

iranzo commented Feb 5, 2020

No progress, I can get the json, I can check it by calling inside function but another thing fails, will keep trying

@iranzo iranzo force-pushed the feat-image-gallery branch from 6c8aa34 to ebdebe4 Compare February 6, 2020 13:32
iranzo and others added 5 commits February 6, 2020 14:36
Signed-off-by: Pablo Iranzo Gómez <Pablo.Iranzo@gmail.com>
Signed-off-by: Pablo Iranzo Gómez <Pablo.Iranzo@gmail.com>
@iranzo iranzo force-pushed the feat-image-gallery branch from ebdebe4 to 058fd49 Compare February 6, 2020 13:37
Signed-off-by: Pablo Iranzo Gómez <Pablo.Iranzo@gmail.com>
@iranzo iranzo force-pushed the feat-image-gallery branch from 058fd49 to 067ec8d Compare February 6, 2020 13:40
@talha131 talha131 closed this Feb 6, 2020
@talha131 talha131 deleted the feat-image-gallery branch February 7, 2020 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants