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

Include ability to caption post heros for blog theme #87

Closed
finleyjb opened this issue Nov 3, 2020 · 11 comments
Closed

Include ability to caption post heros for blog theme #87

finleyjb opened this issue Nov 3, 2020 · 11 comments
Labels
good first issue Good for newcomers

Comments

@finleyjb
Copy link

finleyjb commented Nov 3, 2020

According to Unsplash's API Guidelines, uses of its images must link back to the photographer and attribute unsplash. It would be nice if the blog theme allowed me to attach some sort of frontmatter that allowed me to add a caption to have this attribution.

@laurieontech
Copy link
Contributor

Would certainly be open to a community contribution adding this!

@laurieontech laurieontech added the good first issue Good for newcomers label Nov 19, 2020
@eliasm307
Copy link
Contributor

Hello @laurieontech i would be up for giving this a go, if it's still available.

From my understanding I think there are at least 3 ways to do this, explained below. Would you be able to let me know if I'm on the right track and which solution is preferable? This is one of my first contributions to open source so any advice you have would be appreciated.

For the options below I prefer either option 2 or 3, and just need to know which of these is preferable or if there is a better option before doing too much work on it.

  1. Add a frontmatter field to enter the caption as HTML directly which is then rendered directly using React __dangerouslySetInnerHTML (my least preferred option)

Advantages: caption can be customised by user

Disadvantages: malicious is code could be used for the caption; external tool might be required to sanitise HTML

  1. Add a frontmatter field to enter the caption as markdown which is then rendered as HTML
  1. Adding a caption field in frontmatter that is actually an object with various properties such as authorName, sourceName, and relevant links, then using the values of these fields to conditionally construct the caption text
  • Advantages: requires no external tools, safer?, a bit more straightforward to use for non technical users

  • disadvantages: caption format is fixed which means it might not be suitable in some cases e.g. i18n where the caption template is fixed to one language

I look forward to hearing from you.

@laurieontech
Copy link
Contributor

@eliasm307 thanks so much for picking this up. I’d opt for option 2. This theme is already designed for use with MDX so you should be able to add it to the frontmatter. However, you don’t need to support anything other than plaintext.

What I would recommend is making a separate caption component so that people can shadow it and make changes as they see fit. Then conditionally displaying that should a caption be available for the given hero.

@finleyjb
Copy link
Author

I suppose the caption wouldn't need to support arbitrary HTML or markdown, but it seems like it should at least offer a way to link back to the unsplash/pixaby page or whatever the platform requires you to link to.

@eliasm307
Copy link
Contributor

eliasm307 commented Dec 18, 2020

Hello @laurieontech thank you for your feedback. Making a caption component sounds good.

However I didn't quite understand what you meant by people will be able to "shadow" the component, would you be able to clarify please? My understanding of what you said is that there would be a caption component that would take the caption text as props then the component could be modified as required to style the caption. Is the user modification what you meant by "shadowing"?

Also, regarding your statement about only needing to support plain text, I just want to clarify how this would meet the requirements where there is only one caption field in the markdown. From the link @finjo-the-great included in the issue, the requirement from unsplash is for a caption explicitly with a link to the author's Unsplash profile, an example format is given below:

Photo by Annie Spratt on Unsplash

Apologies but I don't see how something like the above could be generated from a plain text string. I'm taking plain text to mean without any html or markdown so no capability to add hyperlinks. I would assume atleast the caption text and the link to the author would be required, unless if I'm misunderstanding something?

@eliasm307
Copy link
Contributor

How about using 2 fields for the caption, one which would be the caption text, then an optional one for a link which would make the whole caption a hyperlink?

@laurieontech
Copy link
Contributor

Apologies for the delay here, we were off for the holidays.

I hadn't considered the link piece. I think creating a format with specific fields is reasonable. To handle internationalization there should be no English text included. Perhaps something like:
<startCaptionText> <linkName><link> <endCaptionText>.

It only allows for a single link, but it's very configurable.

@eliasm307
Copy link
Contributor

@laurieontech thanks for your reply, hope you had a good break.

I'll look into implementing the solution you described when I get a moment however, I had looked around and found the following in the gatsby contribution guides here Screenshot_20210104-145756_Chrome.jpg

Its a solution for a similar problem however I couldnt find the code for the components with the implementation.

@laurieontech
Copy link
Contributor

Ah, this is the old site code for authoring blog posts. We've since moved to a CMS so I'd have to go digging in git history. But as a pattern it's a perfectly reasonable one to duplicate.

@eliasm307
Copy link
Contributor

@laurieontech I've created a pull request using two frontmatter properties for the caption text and caption link. This was easier and also I think it is more straightforward and flexible to use as the text can be however the user wants using just one markdown field, however, the whole thing becomes a hyperlink if a caption link has been defined, but I don't know if this is actually an issue?

If you strongly prefer the format you had suggested ie , I can work on that but I feel this would be a bit confusing on the markdown side as you need to understand the context of the fields to know the order to fill in fields so the resulting caption comes out in the required order.

Either way, I look forward to your feedback

@LekoArts
Copy link
Contributor

Implemented with #95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants