-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Fix: Allignment issue in the blog page #1029
Fix: Allignment issue in the blog page #1029
Conversation
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.
Welcome to the JSON Schema Community. Thanks a lot for creating your first pull request!! 🎉🎉 We are so excited you are here! We hope this is only the first of many! For more details check out README.md file.
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1029 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 373 373
Branches 94 94
=========================================
Hits 373 373 ☔ View full report in Codecov by Sentry. |
@benjagm Hey Benjamin I tried fixing the blog page alignment and banner image issue and I am successful while doing it in the local system however the build preview above using cloudfare does not align with my changes locally |
pages/blog/index.page.tsx
Outdated
</> | ||
) : ( | ||
frontmatter.authors.map( | ||
(author: any, index: number) => ( |
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.
here please use the correct data type in place of any
pages/blog/index.page.tsx
Outdated
<> | ||
{frontmatter.authors | ||
.slice(0, 2) | ||
.map((author: any, index: number) => ( |
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 use the correct data type in place of any
Thank you for the PR @Ritesh2351235 its working well locally, left few suggestions. |
@DhairyaMajmudar Hey I,m getting some NextJS problems while building the project |
Co-authored-by: Dhairya Majmudar <[email protected]>
@DhairyaMajmudar To handle the type safety in typescript I have added the Author interface in the index.ts file which takes care of |
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.
Working great, LGTM 🚀
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.
@benjagm The changes i have made above are working totally fine on the local machine but are not working on the preview above in cloudfare. |
Comment here can be helpful #1021 (comment) |
Iam pulling the origin to update the branch
@DhairyaMajmudar I made the necessary changes in the tailwind config file but again it's working fine locally but the build preview its not showing the required changes |
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.
Good try, however we should try to avoid using the tailwind safelist that way.
Let's make sure we follow the best practices for avoid our styles to be purged:
https://v2.tailwindcss.com/docs/optimizing-for-production#writing-purgeable-html
tailwind.config.js
Outdated
@@ -4,6 +4,28 @@ module.exports = { | |||
'./components/**/*.{js,ts,jsx,tsx,md}', | |||
], | |||
darkMode: "class", | |||
safelist: [ |
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.
Rather than adding elements to the safelist here we should check for other alternatives like making sure we are adding styles without using string concatenation to create class names:
https://v2.tailwindcss.com/docs/optimizing-for-production#writing-purgeable-html
@benjagm Thank you!! |
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.
I just left a comment to see if we can avoid using the tailwind safelist config.
tailwind.config.js
Outdated
'h-8', | ||
'w-8', | ||
'h-11', | ||
'w-11', | ||
'bg-slate-50', | ||
'rounded-full', | ||
'-ml-3', | ||
'bg-cover', | ||
'bg-center', | ||
'border-2', | ||
'border-white', | ||
], |
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.
Are we sure we need to add this to the safelist? I think we can avoid this.
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.
I have removed the safelist from the tailwindconfig now
Its working fine!
@Ritesh2351235 Can we implement the same change on the landing page? There is a preview of the last blog post, and it will be great to have the same behavior. As soon as we add this, we are ready to merge. Great job! |
@benjagm I have corrected the Blog component of the Landing page which aligns similarly to the blog page. |
Hey @benjagm, |
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.
Great job!
Congratulations, @Ritesh2351235 for your first pull request merge in this repository! 🎉🎉. Thanks for your contribution to JSON Schema! |
What kind of change does this PR introduce?
Fix for the Blog page alignment bug and recover the banner image for the latest GSOD blog.
Issue Number:
Screenshots/videos:
Summary
Resized the author avatar sizes for more than 2 authors for a single blog which solves the banner issue and alignment issue.
Also changes the author name format for more than 2 authors on a single blog which could be seen in the image
Does this PR introduce a breaking change?
No