Skip to content

scripts(email_issue): add links in author - #82

Merged
gupta-ji6 merged 3 commits into
mainfrom
update-issueEmailGenerator-script
Aug 7, 2022
Merged

scripts(email_issue): add links in author#82
gupta-ji6 merged 3 commits into
mainfrom
update-issueEmailGenerator-script

Conversation

@gupta-ji6

@gupta-ji6 gupta-ji6 commented Dec 4, 2021

Copy link
Copy Markdown
Member

Description

TODO

  • Fix comma placement for multiple authors
  • Fix for the condition when authors are null

@gupta-ji6 gupta-ji6 added the enhancement New feature or request label Dec 4, 2021
@gupta-ji6 gupta-ji6 self-assigned this Dec 4, 2021
@gupta-ji6
gupta-ji6 requested review from a team and prateek3255 and removed request for a team December 4, 2021 09:23
Comment thread scripts/issueEmailGenerator.js Outdated
const authorsWithWebsite = authors.map((author, index) => {
const isLastElement = index === authors.length - 1;
console.log(isLastElement);
return `[${author.Name}](${author.Website})` + (isLastElement ? '' : `, `);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It can just be this -

Suggested change
return `[${author.Name}](${author.Website})` + (isLastElement ? '' : `, `);
return `[${author.Name}](${author.Website})`;

Comment thread scripts/issueEmailGenerator.js Outdated
return `[${author.Name}](${author.Website})` + (isLastElement ? '' : `, `);
});

return `*by ${authorsWithWebsite}*`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And then joining like this should do the job I believe

Suggested change
return `*by ${authorsWithWebsite}*`;
return `*by ${authorsWithWebsite.join(',')}*`;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The VS Code Markdown Preview was confusing me as it was showing me two commas, but in reality, there was only one. 😏

@gupta-ji6
gupta-ji6 marked this pull request as ready for review December 23, 2021 19:02
@gupta-ji6
gupta-ji6 requested a review from a team as a code owner December 23, 2021 19:02
@gupta-ji6

Copy link
Copy Markdown
Member Author

@prateek3255 review this as well if you have time, have resolved all review comments

@gupta-ji6
gupta-ji6 force-pushed the update-issueEmailGenerator-script branch from fe0283c to be3ea82 Compare August 6, 2022 16:13

# This week in GIF

[${currentIssue.gif.caption}](${process.env.SITE_URL}issues/${options.issueNumber}?section=gif)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gupta-ji6 Why did we remove the gif image and replaced with Link?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

in #69, we replaced all the gifs media with videos & videos weren't supported to be sent in Buttondown if I remember correctly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh yeah, can't do video in emails. I guess we would need to still figure out a way to keep two versions, video for the web and gif for the email, because just adding a link doesn't look good. Although this can be done in a separate PR, so let's bring this in for now

@gupta-ji6
gupta-ji6 merged commit c29e078 into main Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants