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
refactor: replace PRSocialCard
with githubs Opengraph image
#889
Conversation
β Deploy Preview for design-insights ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
β Deploy Preview for oss-insights ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Compliance Checks Passed!
lib/utils/github.ts
Outdated
@@ -20,7 +20,7 @@ const generateApiPrUrl = ( | |||
const trimmedUrl = url.trim(); | |||
const githubUrl = new URL(trimmedUrl.includes("https://") ? trimmedUrl : `https://${trimmedUrl}`); | |||
const { pathname } = githubUrl; | |||
|
|||
console.log(pathname); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stray
next.config.js
Outdated
@@ -7,7 +7,8 @@ module.exports = { | |||
"images.unsplash.com", | |||
"www.github.com", | |||
"github.com", | |||
"res.cloudinary.com" | |||
"res.cloudinary.com", | |||
"opengraph.githubassets.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't optimise github images: https://nextjs.org/docs/basic-features/image-optimization#domains
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Fixed |
const { isValid, url } = generateGhOgImage(githubLink); | ||
|
||
return ( | ||
// eslint-disable-next-line @next/next/no-img-element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please stop disabling eslint rules and state your problems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nextjs warning to not use the native img
tag in next https://nextjs.org/docs/messages/no-img-element
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems aggressive for linter, but disabling it seems like a band-aid. Are there other alternatives @OgDev-01?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another alternative is to disable globally in the .eslintrc
rules
Or try this solution from the official next docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Need to resolve solution on lint rule
π This PR is included in version 1.30.0-beta.8 π The release is available on GitHub release Your semantic-release bot π¦π |
## [1.30.0](v1.29.0...v1.30.0) (2023-02-21) ### π Features * add highlights to user profile page ([#859](#859)) ([4b01376](4b01376)), closes [#830](#830) * add dropdown for topics in Explore ([#877](#877)) ([9579902](9579902)), closes [#533](#533) * disable create insight page button if the page name is missing ([#857](#857)) ([95da564](95da564)), closes [#852](#852) ### π Bug Fixes * disable create page button only if insight page name is empty ([#893](#893)) ([ed52399](ed52399)), closes [#892](#892) * hide non-functional elements in highlights card ([#881](#881)) ([ba1bd5e](ba1bd5e)) * rename 404 image file ([4e5171f](4e5171f)) * replace `PRSocialCard` component with GitHub OpenGraph image ([#889](#889)) ([3bfc5a4](3bfc5a4)), closes [#883](#883) * reset text input fields when clear button is clicked ([#869](#869)) ([783098d](783098d)), closes [#858](#858) * user profile interest pill UI fix ([#890](#890)) ([4cbff36](4cbff36))
π This PR is included in version 1.30.0 π The release is available on GitHub release Your semantic-release bot π¦π |
## [1.30.0-beta.8](open-sauced/app@v1.30.0-beta.7...v1.30.0-beta.8) (2023-02-21) ### π Bug Fixes * replace `PRSocialCard` component with GitHub OpenGraph image ([#889](open-sauced/app#889)) ([3bfc5a4](open-sauced/app@3bfc5a4)), closes [#883](open-sauced/app#883)
## [1.30.0](open-sauced/app@v1.29.0...v1.30.0) (2023-02-21) ### π Features * add highlights to user profile page ([#859](open-sauced/app#859)) ([4b01376](open-sauced/app@4b01376)), closes [#830](open-sauced/app#830) * add dropdown for topics in Explore ([#877](open-sauced/app#877)) ([9579902](open-sauced/app@9579902)), closes [#533](open-sauced/app#533) * disable create insight page button if the page name is missing ([#857](open-sauced/app#857)) ([95da564](open-sauced/app@95da564)), closes [#852](open-sauced/app#852) ### π Bug Fixes * disable create page button only if insight page name is empty ([#893](open-sauced/app#893)) ([ed52399](open-sauced/app@ed52399)), closes [#892](open-sauced/app#892) * hide non-functional elements in highlights card ([#881](open-sauced/app#881)) ([ba1bd5e](open-sauced/app@ba1bd5e)) * rename 404 image file ([4e5171f](open-sauced/app@4e5171f)) * replace `PRSocialCard` component with GitHub OpenGraph image ([#889](open-sauced/app#889)) ([3bfc5a4](open-sauced/app@3bfc5a4)), closes [#883](open-sauced/app#883) * reset text input fields when clear button is clicked ([#869](open-sauced/app#869)) ([783098d](open-sauced/app@783098d)), closes [#858](open-sauced/app#858) * user profile interest pill UI fix ([#890](open-sauced/app#890)) ([4cbff36](open-sauced/app@4cbff36))
What type of PR is this? (check all applicable)
Description
This PR:
Related Tickets & Documents
Fixes #883
Mobile & Desktop Screenshots/Recordings
Added tests?
Added to documentation?
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?