Topic List Previews Theme Component

Three challenges:

  • Im fairly sure thumbnails are not serialised here. Feel free to confirm. That could be overcome by enhancing the sidecar plugin.
  • when I last looked at this the structure of the page did not permit low risk overrides isolated to a leaf template. Generally we don’t want to override the whole page which might cause breaking changes or mask feature updates from core. A PR to core might help here …
  • Space?

Feel free to submit required PRs

1 Like

Fair points. I might choose another presentation for the category pages, maybe boxed or something like that.

I have another question.
Is it possible to have category pages (which contains topic lists) being displayed in tiles on mobile, but with thumbnails on desktop? I tried several settings tweaks but I couldn’t achieve this.


edit: I achieved it. I’ll detail how I did it a bit later in case it could interest someone else, I added this edit just so nobody spends time explaining me how to do it :wink:

So, If I’m not mistaken, here are the settings…

First we need the tiles display only on the mobile view of (at least?) a topic list:

Then the thumbnails for desktop AND mobile:


I was mistaked at first because I thought “tiles” and “thumbnails” were two different presentations of a topic list, both containing the images. but this is not the case. Thumbnails are needed if you want images to appear on a topic list, whether it’s a tile or not.

There is no need to manually add your categories here:
image

Since we’ll override this setting by enabling the following one:

Now, you should have small thumbnails on desktop in any topic list (latest, a specific category, etc), and a “tiled” presentation on mobile with large thumbnails:

1 Like

I have a little issue with my user profile, I guess this is a small bug:

Since it mentions the thumbnail, I suppose this is related to this theme component.

Still playing with this theme component by the way… And it’s awesome. :+1:


I have a question. Is it possible to prevent a topic from having a thumbnail, though the topic actually contains some?

Here’s my case:
I have a documentation category for which thumbnails are welcome.
But I also have a topic that just gives general advice when creating a new topic:

There is no meaningful image in it, but it adds a thumbnail automatically:

The only way I see to circumvent the issue is adding a random image at some point in the topic, and setting it as the thumbnail.

Example:

(but I admit it looks nice though…)

1 Like

Oh yes that localisation is failing and needs to be fixed. :beetle:

No, if there’s an image it will try to use one. Adding an additional nice one is the perfect workaround :+1:

This will look better for the consistency of your page layout too.

In any case I often use a featured topics Component (the built-in TLP one or another) to summarise those kind of posts at the top so having an image is quite nice.

1 Like

I am trying to restrict the thumbnail and tiles to a specific tag in addition to certain categories but it is not working for tags. Here is my settings page i only want tiles and thumbnails for featured tag. Can you please tell me if I am doing something wrong.

There’s also a missing string in the plugin’s settings:

image

And another bug with the topic list excerpt removes links option.
So, as already reported, if I disable it there is no more link at all, even their text, as well as the “read more” button:

If I enable it, links in excerpts appear, as well as the “read more” link, but for some reason,

  1. The read more link isn’t styled as a link (also already reported, but since it’s all related to the same option, I prefer to group all the issues at once)

  2. Some excerpts are wrongly wrapped. Some examples:
    Only the first part of the sentence from the excerpt is wrapped here:


    An empty line is wrapped as an excerpt:

I’d be happy to help more but unfortunately, I don’t know anything about plugins and most of Discourse’s code… :pensive:

1 Like

that looks like a bug, I’ll have a look soon.

3 Likes

I’ve fixed this on the beta branch, could you please confirm things seem to be good now then I will merge.

For this to work you have to remove tag and tag-mobile and add the specific tag to the tag list setting.

(Install the beta version as another component and associate it with a test theme).

It turned out to be a breaking change in core.

Use the advanced button to reveal the branch and type in beta:

If I don’t hear from you within a while I’ll merge it anyway, but here is your chance to test and bring that forward.

1 Like

This is only gained by doing :hammer_and_wrench: , a little bit at a time and picking up pieces of knowledge along the way :nerd_face: :open_book:

3 Likes

There’s also a JS error when we navigate on the website if the theme component is installed.

It’s also visible on your own website: https://starzen.space/
Click on any post and look at the JS console.

TypeError: Cannot read properties of null (reading 'querySelector')

The error is triggered on this Discourse file line:

1 Like

Looks like a breaking change in core: FIX: Don't listen for focus/blur events if the topic-list opts out of… · discourse/discourse@97e7bb1 · GitHub

Mainly fixed with: COMPATIBILITY: add css class to tiles to support focus · merefield/discourse-topic-list-previews-theme@4f0f0f0 · GitHub

Fix on beta branch as demo’d on my site.

Advantage of fixing this the way is we get the last visited indicator on the tile now :+1:

(Most of the errors disappears in mobile mobile too, but this indicator is not normally visible on mobile in any case, so I’m going to call this fixed!)

I will need to follow up on the less frequent error related to the title element.

1 Like

This is fixed in beta: FIX: missing localisation on user prefs and update locale paths

image

1 Like

Thanks!

Regarding the plugin topic list excerpt remove links option, may I suggest changing the regex you currently use?

URI::regexp will remove any string that has a link pattern, which is not, in my opinion, always desirable.
Links have sometimes the same text content as their href value. That’s even a very common case in any forum, and removing the text content can lead to bizarre excerpts with sentences that make no sense.
This regex will also remove words that are inside or outside links sometimes. I give examples below.

Here’s a comparison with another regex: <a .+?\>(.+)?<\/a>
This is just an example regex; I understand that URI::regexp covers links in a more complex (and supposedly more reliable) way.

Here’s an example post:

And the resulting excerpts in the topic list:

URI::regexp

image
(the facebook link text content is removed, and it also removed the word “Day” in “Astronomy Picture of the Day” for an unknown reason :face_with_raised_eyebrow:)

<a .+?\>(.+)?<\/a>

image


Another example, just about a link text content removed that makes the excerpt weird to read:

URI::regexp

image
(“the link was This may be my favorite halo”…?)

<a .+?\>(.+)?<\/a>

image
(now that makes sense)

So, I’m not suggesting <a .+?\>(.+)?<\/a> is better, it was just a quick test, but I’m not sure using URI::regexp is the best choice regarding the results, both for the two reasons I mentioned: link text content vanishes, making excerpts weird sometimes, and also it removes words inside or outside links from time to time for an obscure reason. The latter issue seems frequent enough not to be negligible.

3 Likes

I appreciate the downsides of the current method (including the surprisingly weird over enthusiasm of the current algorithm where it deletes too much) but we arrived here through experience.

The main reason this option exists is actually to:

  • preserve formatting of preview when links are very long (ie delete all links so a long link will never leak into the excerpt to cause problems). Try the scenario where a link is very very long. The old support topic is littered with issues with Topic posts with long links.
  • preserve the excerpt as a predictable click surface where it will always navigate to the Topic. The text is too small for this to be discretionary.

Also should be:

  • simple to maintain and not cause support noise. Because the current method is a supported utitlity class I don’t have to worry about maintaining it.

I’m not yet convinced we need to change this for the general public?

I might be open to making this a selection of three options, OFF, no links (ie current method) and experimental? PR welcome. Let me move the current sidecar code to the master plugin branch first. I’ll try to do that this week.

3 Likes

Thank you for your reply :slight_smile:

I fixed the last sentence of my previous post, I forgot a word…
I wrote

and also it removes words inside or outside links from time to time for an obscure reason. The latter issue seems frequent enough to be negligible."

I forgot a “not”, so…

and also it removes words inside or outside links from time to time for an obscure reason. The latter issue seems frequent enough not to be negligible.


I might not understand much code stuff but I’ll try to understand and fix the issue number 2 I mentioned here: Topic List Previews Theme Component - #110 by Canapin today.

(the excerpts aren’t properly wrapped by .topic-excerpt: the wrapper seems to close just before the first link present in the excerpt instead of the end of the excerpt)

Tbh the best approach might be we raise an issue with the maintainer of that utility and get them to fix it? It’s certainly not behaving as you’d expect?

Yeah haven’t looked at that yet. Feel free to PR a fix in meantime.

1 Like

Thanks, I haven’t thought of that, I had no idea where this utility came from.

After searching for a bit, basically the regex recognizes pretty much any word or string as a part of a URI scheme as long as it’s immediately followed by a colon -ie: no space in between- which is understandable, but also a bit too much as in the English language (contrary to the French language, for example), we do not put space between a word and a colon. Hence “legit” words being eaten by the regex just because they’re unfortunately followed by a colon.


A fix could be having a field (not pre-filled to prevent any harm in existing installations) in the plugin settings to enter the scheme(s) we want to remove.

For example, the setting could be:

URI schemes to remove: http|https|ftp|mailto

That would result in:

#{URI::regexp(['http', 'https', 'ftp', 'mailto'])}
(but it’s, unfortunately, case sensitive, but that can surely be tweaked in some way)

If the setting is empty, then this is what would be used:

#{URI::regexp}

Being the current behavior.

Would a pull request with such a setting be welcomed?

Thank you for your work! It works fine on pc. Can I get the thumbnails to not show full width on mobile devices (phones)?just like Pinterest does.
image

2 Likes

You could experiment with the css. That’s going to make each column too small to fit all the elements without distortion though I suspect.

1 Like

Thanks, I’m going to try it.