Skip to content

Commit

Permalink
FEATURE: Allow post/topic thumbnails to be prioritized via markdown (#…
Browse files Browse the repository at this point in the history
…12044)

Previously we would always take the first image in a post to use as the
thumbnail. On media-heavy sites, users may want to manually select a
specific image as the topic thumbnail. This commit allows this to be
done via a `|thumbnail` attribute in markdown.

For example, in this case, bbb would be chosen as the thumbnail:

```
![alttext|100x100](upload://aaa)
![alttext|100x100|thumbnail](upload://bbb)
```
  • Loading branch information
davidtaylorhq committed Feb 11, 2021
1 parent a6bb7e6 commit 830797a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
1 change: 1 addition & 0 deletions app/assets/javascripts/pretty-text/addon/allow-lister.js
Expand Up @@ -190,6 +190,7 @@ export const DEFAULT_LIST = [
"img[height]",
"img[title]",
"img[width]",
"img[data-thumbnail]",
"ins",
"kbd",
"li",
Expand Down
Expand Up @@ -177,7 +177,7 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
const token = tokens[idx];
const alt = slf.renderInlineAsText(token.children, options, env);
const split = alt.split("|");
const altSplit = [];
const altSplit = [split[0]];

// markdown-it supports returning HTML instead of continuing to render the current token
// see https://github.com/markdown-it/markdown-it/blob/master/docs/architecture.md#renderer
Expand All @@ -195,7 +195,7 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
}

// parsing ![myimage|500x300]() or ![myimage|75%]() or ![myimage|500x300, 75%]
for (let i = 0, match, data; i < split.length; ++i) {
for (let i = 1, match, data; i < split.length; ++i) {
if ((match = split[i].match(IMG_SIZE_REGEX)) && match[1] && match[2]) {
let width = match[1];
let height = match[2];
Expand Down Expand Up @@ -238,6 +238,8 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
}
} else if ((data = extractDataAttribute(split[i]))) {
token.attrs.push(data);
} else if (split[i] === "thumbnail") {
token.attrs.push(["data-thumbnail", "true"]);
} else {
altSplit.push(split[i]);
}
Expand Down
12 changes: 9 additions & 3 deletions lib/cooked_post_processor.rb
Expand Up @@ -483,14 +483,20 @@ def create_link_node(klass, url, external = false)

def update_post_image
upload = nil
eligible_image_fragments = extract_images_for_post
images = extract_images_for_post

# Loop through those fragments until we find one with an upload record
@post.each_upload_url(fragments: eligible_image_fragments) do |src, path, sha1|
@post.each_upload_url(fragments: images.css("[data-thumbnail]")) do |src, path, sha1|
upload = Upload.find_by(sha1: sha1)
break if upload
end

if upload.nil? # No specified thumbnail. Use any image:
@post.each_upload_url(fragments: images.css(":not([data-thumbnail])")) do |src, path, sha1|
upload = Upload.find_by(sha1: sha1)
break if upload
end
end

if upload.present?
@post.update_column(:image_upload_id, upload.id) # post
if @post.is_first_post? # topic
Expand Down
13 changes: 13 additions & 0 deletions spec/components/cooked_post_processor_spec.rb
Expand Up @@ -772,6 +772,19 @@
end
end

it "prioritizes data-thumbnail images" do
upload1 = Fabricate(:image_upload, width: 1750, height: 2000)
upload2 = Fabricate(:image_upload, width: 1750, height: 2000)
post = Fabricate(:post, raw: <<~MD)
![alttext|1750x2000](#{upload1.url})
![alttext|1750x2000|thumbnail](#{upload2.url})
MD

CookedPostProcessor.new(post, disable_loading_image: true).post_process

expect(post.reload.image_upload_id).to eq(upload2.id)
end

context "post image" do
let(:reply) { Fabricate(:post_with_uploaded_image, post_number: 2) }
let(:cpp) { CookedPostProcessor.new(reply) }
Expand Down

3 comments on commit 830797a

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/topic-list-thumbnails-theme-component/150602/126

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/image-scaling-not-working-if-alt-text-is-missing/179883/5

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/set-the-image-used-for-social-network-sharing/107880/30

Please sign in to comment.