Show Images in posts in an image grid

[MarekBenjamin]

Hello, this is just a very first draft for this feature. I hope that we can discuss this PR and improve it over time.

I still have a lot of questions, mainly concerning where things can be done.

As you see, in this first attempt, I basically remove all images from a post and reinsert them afterwards using the new image_grid.tpl.

The point I am most challenged / confused at the moment is the redundancy of formated-html in the item / body variable , which seems to be present from the very beginning of the call stack in Item:preparaBody() method, and the BBcode used in parallel.

[MarekBenjamin]

You will be able to have a look at the current status via my development instance at:
https://friendica-dev.mbbit.de/profile/friendicadev

[MrPetovan]

The rendered-html key of the $item array is the cached HTML version of the post BBCode body. We try to perform as few BBCode::convert calls because they are expensive. This is done in Item::putInCache() which stores the result in the item table.

It looks pretty good already!

[loma-one]

Hi @MarekBenjamin
This looks really good.
Following hint: I use Friendica among other things also as a blog and / or want to place different images at a certain position in the text. This should still be possible please.

[MarekBenjamin]

@loma-one Thank you very much for the feedback. I was already made aware about this in this Friendica Support Thread. This will also be one of my questions for going on with this feature:

How to distiguish between embded images and attached

But I will itemize a few more questions within the next day(s) and post them here altogether.

[tobiasd]

I basically remove all images from a post and reinsert them afterwards

Do I understand you correct, that when I craft a blog like entry putting the images at the correct places in the text, this would mean that my posting would look totally different once it is posted, because all the images would then be at the end of the posting?

github had a hickup? I just got the question by @loma-one and your reply shown... sorry for the double question then.

[MarekBenjamin]

@tobiasd yep, currently this is the case, and one (among many others) of the reasons why it is a draft PR. So at the moment is really only a hack for a proof of concept, which is nevertheless very valuable for me to learn about the code and design patterns.

The whole process of searching in the cached HTML for <a><img></a> and removing it, is for sure not the final solution. I am perfectly aware about this. In fact, this is a second question for the list of questions:

How and where to best modify the post, at BBCode level or in rendered HTML

@MrPetovan partially answered already: #12334 (comment)

I might figure it out by my self, but any feedback and inspirations are highly appreciated 👼

[MrPetovan]

You should modify the post only at the HTML render. When editing a post users should see exactly what BBCode they entered.

[MarekBenjamin]

You should modify the post only at the HTML render. When editing a post users should see exactly what BBCode they entered.

I see, things start to become clearer. 😄 Are you able to judge if it is better to create the image grid:

• directly in Item::putInCache() or
• better alternating the rendered-html in $item afterwards? (May be register a hook for Hook::callAll('prepare_body', $hook_data);?

[MrPetovan]

It depends on the transformation. If it is a localized transformation related to a BBCode tag, please add your changes to BBCode::convert. If you mean to rearrange the post's HTML after BBCode conversion without any database or remote HTTP calls, you can do it in Item::prepareBody.

I would suggest against a hook, they are needed for optional addons but it sounds like your work would benefit everybody.

Also please make sure your changes also work in the vier theme. I recommend you create a delegated account using the vier theme you can switch to and back from your frio-using account, it's easier to switch account than switch theme.

[MarekBenjamin]

Regarding the attachment issue: This is the bbcode from $item['body'] (taken from here)

This post is to test whether the inline images are kept in place and not manipulated by the new image grid feature. 
Thus, the following image should be there: 

[url=...][img=...][/img][/url]

After this lines the attached images follow. 
These four images are expected to be included in an 2x2 image grid 

[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]

and the $item['rendered_html] is

<div class="wall-item-body e-content " id="wall-item-body-195" dir="auto">
  This post is to test whether the inline images are kept in place and not manipulated by the new image grid feature. 
  Thus, the following image should be there:
  <br>
  <a href="..."><img src="..." alt="" title=""></a>
  <br><br>
  After this lines the attached images follow. These four images are expected to be included in an 2x2 image grid
  <br><br><br><
  <a href="..."><img src="..." alt="" title=""></a>
  <br><br>
  <a href="..."><img src="..." alt="" title=""></a>
  <br><br>
  <a href="..."><img src="..." alt="" title=""></a>
</div>

So, as you see, there is really little semantic available to distinguish between 'inline' images to be kept in place and the 'attached' images I would like to put in an image grid.

What are your opinions? I see the following possibilities:

1. Naive: Just search backwards through the DOM tree until a "Text String" is encountered
2. Add semantic to $item['body'], e.g.

These four images are expected to be included in an 2x2 image grid 
**[attachment]**
[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]
[url=...][img=...][/img][/url]
**[/attachment]**

3. Add semantic to $item['rendered_html], e.g. adding

  After this lines the attached images follow. These four images are expected to be included in an 2x2 image grid
**<div class=""wall-item-body e-content attachments">**
  <a href="..."><img src="..." alt="" title=""></a>
  <br><br>
  <a href="..."><img src="..." alt="" title=""></a>
  <br><br>
  <a href="..."><img src="..." alt="" title=""></a>
**</div>**

and of course:

4. YOUR IDEA

Please comment, and if 2. or 3. is favorable: Where should the semantic injection be done?

Thank you for your feedback!

[annando]

Wouldn't it be much easier, when the whole process was done at an earlier stage and not after the rendering?

To clarify this: We already are storing received images from remote networks not directly in the body of the message but as attached media. On incoming Friendica posts (no matter if remote or local) we should easily be able to detect if there were several image links at the bottom of the post. Then we can strip the images from the body. This means that they then will automatically be added via the already existing process of attaching media to the post while rendering.

[MarekBenjamin]

To clarify this: We already are storing received images from remote networks not directly in the body of the message but as attached media.

Hi @annando , thank you! That was such an important clarification for me, now I get this "attachment" topic finally 👼
Can you please provide the code location, e.g., for mastodon messages?

On incoming Friendica posts (no matter if remote or local) we should easily be able to detect if there were several image links at the bottom of the post.

and also this location where "incoming" friendica messages are processed?

Thank you so much!

[annando]

@MarekBenjamin the pictures are currently added in the function addVisualAttachments in src/Model/Item.php. There the content is added using the content/image.tpl template. I guess you have to make a change there. The rest (removing content from the body) does need some more "insider knowledge", so I guess I will work on it the next days. But you can easily test on Mastodon posts or posts that had been done via the API,

[MarekBenjamin]

Thank you so much for offering your support! ❤️

My problem for at least the friendica post is that the images are already included both in $item['body'] and $item['rendered_html'] before addVisualAttachments() is called. In fact, inside addVisualAttachments() there are a couple of conditions checking if the images are already present.

I already pushed here an image grid wrapper template view/templates/content/image_grid.tpl which puts the images in content/image.tpl using the sketch function make_image_grid(array &$data)

The issue is to remove correctly the previously present images in $item['body'] and $item['rendered_html'] so that they not appear twice (old embedded and in the added grid), see here for the current status:
https://friendica-dev.mbbit.de/display/fa8fe840-2863-8e8b-e42c-410001128098

[MarekBenjamin]

Currently, I am elaborating a DOM manipulation for $item['rendered_html'] to detect the last occurrences of HTML tags containing values, i.e. text, and remove the a and img tags afterwards

[MarekBenjamin]

I think a general question which needs to be answered is

When should media be conisered as attached and when part of message layout

I think the idea of "all images at the end are assumed to be attached" is already quite good, only that this can be confusing when someone wants to add a "closing image" to its message.
Therefore, I actually prefer to have some kind of semantic expression for attachments on the long run.
This could be as a first step be as simple as a double blank line.

[annando]

The change that I want to perform is to remove all images that are added to the end of the body from the body and to add them in the mentioned function. Since images could be posted intentionally at some places, we mustn't remove these images.

So the general rule is to not touch the body and the rendered html in any case. Posts from Mastodon (and posts generated via the API) for example shouldn't have image links inside the body. The media in these posts is normally added only in the mentioned function.

[MarekBenjamin]

FYI: Added image grid creation to the addVisualAttachements method. Still needs testing to cover different cases of landscape / portrait mixes. But it already works quite well, leaving the images between text untouched and creating an image grid for the images in the end:

https://friendica-dev.mbbit.de/display/fa8fe840-2863-8e8b-e42c-410001128098

[MarekBenjamin]

I think we can start the review phase. I tried a couple of different landscape and portrait mixes and implemented cases to best present those mixes so that both columns are as equal as possible:

More landscapes than portraits
Equal amount of landscapes and portraits
More portraits than landscapes

If you want to support this PR you can - if you are able - check out this branch on your dev instance and play around adding multiple different aspects images and tell me when something is still not handles correctly.

Cheers!

[MrPetovan]

Thank you for your work and your reactivity!