-
Notifications
You must be signed in to change notification settings - Fork 2
Eleino/enhancement/596 update news card and single newspage rendering logic #601
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
Eleino/enhancement/596 update news card and single newspage rendering logic #601
Conversation
Skoivumaki
left a comment
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 work! Nice to see the additional "error catches" you have implemented for edge-case scenarios.
- Not part of the issue but, could you change the news text to be aligned from the left instead of center. (its Figma Design and I believe it will make text more readable)
- Still not part of the issue: Please slightly increase the
line-heightof text inNewsElementPagetext/pand decrease container width for desktop devices. (likewidth: 80%and test on all devices) - Change
noImageContainerclassheightto2vhor similar to remove the odd padding. (test on all devices to ensure it looks good) Example with these changes on right:
…rea, and slightly increased line height of the text
| text-align: center; | ||
| width: 100%; | ||
| text-align: left; | ||
| width: 80%; |
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.
Add breakpoint to keep width: 98% (to add very slight padding) on mobile and PR is good 👍
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.
Done!
Rutjake
left a comment
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! The news items without a picture look much better now.
I won't merge this yet because I need to make some minor changes unrelated to the issue.
Restores lost SEO codes related to conflicting merges or accidental deletions on GitHub, specifically: - **Restores** enhanced news and gallery page rendering logic. - **Improves/Restores** SEO metadata handling (titles, descriptions, keywords). - **Refactors** Open Graph image retrieval and URL formatting for better social sharing.
📄 Pull Request Overview
Closes #596
🔧 Changes Made
✅ Checklist Before Submission
console.log()or other debugging statements are left.📝 Additional Information
Provide any additional context or information that reviewers may need to know: