-
-
Notifications
You must be signed in to change notification settings - Fork 900
fix: redesign 'Latest News' section in Community Newsroom #3982
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
Conversation
WalkthroughThe changes involve modifications to the JSX structure and class attributes in the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 180000ms (3)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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 AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
components/newsroom/Newsroom.tsx (1)
24-129
: 🛠️ Refactor suggestionOverall code improvements align with PR objectives but need Prettier fixes.
The changes successfully implement the redesign of the "Latest News" section with improved layout and alignment as per the PR objectives. The card-based layout, better organization, and responsive design improvements are evident in the code. However, all the formatting changes violate the project's Prettier rules and will cause pipeline failures.
I recommend running
npm run format
oryarn format
(whichever is used in the project) to automatically fix all the Prettier issues before merging this PR.🧰 Tools
🪛 ESLint
[error] 24-27: Replace
⏎··········typeStyle={ParagraphTypeStyle.md}⏎··········className='mx-auto·mt-5·max-w-2xl'⏎········
with·typeStyle={ParagraphTypeStyle.md}·className='mx-auto·mt-5·max-w-2xl'
(prettier/prettier)
[error] 28-29: Replace
⏎··········a·blog·post?·We·love·community
with·a·blog·post?·We·love·community⏎·········
(prettier/prettier)
[error] 30-33: Replace
⏎············href='https://github.com/asyncapi/website/issues/new?template=blog.md'⏎············target='_blank'⏎··········
with·href='https://github.com/asyncapi/website/issues/new?template=blog.md'·target='_blank'
(prettier/prettier)
[error] 41-44: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 66-69: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 76-80: Replace
⏎··············href='https://twitter.com/AsyncAPISpec'⏎··············className='mt-4·inline-flex·items-center'⏎··············target='_blank'⏎············
with·href='https://twitter.com/AsyncAPISpec'·className='mt-4·inline-flex·items-center'·target='_blank'
(prettier/prettier)
[error] 102-105: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 109-110: Delete
⏎···········
(prettier/prettier)
[error] 113-117: Replace
⏎··············href='https://www.youtube.com/c/AsyncAPI'⏎··············className='mt-4'⏎··············target='_blank'⏎············
with·href='https://www.youtube.com/c/AsyncAPI'·className='mt-4'·target='_blank'
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 24-24: Replace
⏎··········typeStyle={ParagraphTypeStyle.md}⏎··········className='mx-auto·mt-5·max-w-2xl'⏎········
with·typeStyle={ParagraphTypeStyle.md}·className='mx-auto·mt-5·max-w-2xl'
prettier/prettier
[error] 28-28: Replace
⏎··········a·blog·post?·We·love·community
with·a·blog·post?·We·love·community⏎·········
prettier/prettier
[error] 30-30: Replace
⏎············href='https://github.com/asyncapi/website/issues/new?template=blog.md'⏎············target='_blank'⏎··········
with·href='https://github.com/asyncapi/website/issues/new?template=blog.md'·target='_blank'
prettier/prettier
[error] 41-41: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 66-66: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 76-76: Replace
⏎··············href='https://twitter.com/AsyncAPISpec'⏎··············className='mt-4·inline-flex·items-center'⏎··············target='_blank'⏎············
with·href='https://twitter.com/AsyncAPISpec'·className='mt-4·inline-flex·items-center'·target='_blank'
prettier/prettier
[error] 102-102: Replace
⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 109-109: Delete
⏎···········
prettier/prettier
[error] 113-113: Replace
⏎··············href='https://www.youtube.com/c/AsyncAPI'⏎··············className='mt-4'⏎··············target='_blank'⏎············
with·href='https://www.youtube.com/c/AsyncAPI'·className='mt-4'·target='_blank'
prettier/prettier
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/newsroom/Newsroom.tsx
(3 hunks)
🧰 Additional context used
🪛 ESLint
components/newsroom/Newsroom.tsx
[error] 24-27: Replace ⏎··········typeStyle={ParagraphTypeStyle.md}⏎··········className='mx-auto·mt-5·max-w-2xl'⏎········
with ·typeStyle={ParagraphTypeStyle.md}·className='mx-auto·mt-5·max-w-2xl'
(prettier/prettier)
[error] 28-29: Replace ⏎··········a·blog·post?·We·love·community
with ·a·blog·post?·We·love·community⏎·········
(prettier/prettier)
[error] 30-33: Replace ⏎············href='https://github.com/asyncapi/website/issues/new?template=blog.md'⏎············target='_blank'⏎··········
with ·href='https://github.com/asyncapi/website/issues/new?template=blog.md'·target='_blank'
(prettier/prettier)
[error] 41-44: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 66-69: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 76-80: Replace ⏎··············href='https://twitter.com/AsyncAPISpec'⏎··············className='mt-4·inline-flex·items-center'⏎··············target='_blank'⏎············
with ·href='https://twitter.com/AsyncAPISpec'·className='mt-4·inline-flex·items-center'·target='_blank'
(prettier/prettier)
[error] 102-105: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
(prettier/prettier)
[error] 109-110: Delete ⏎···········
(prettier/prettier)
[error] 113-117: Replace ⏎··············href='https://www.youtube.com/c/AsyncAPI'⏎··············className='mt-4'⏎··············target='_blank'⏎············
with ·href='https://www.youtube.com/c/AsyncAPI'·className='mt-4'·target='_blank'
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/newsroom/Newsroom.tsx
[error] 24-24: Replace ⏎··········typeStyle={ParagraphTypeStyle.md}⏎··········className='mx-auto·mt-5·max-w-2xl'⏎········
with ·typeStyle={ParagraphTypeStyle.md}·className='mx-auto·mt-5·max-w-2xl'
prettier/prettier
[error] 28-28: Replace ⏎··········a·blog·post?·We·love·community
with ·a·blog·post?·We·love·community⏎·········
prettier/prettier
[error] 30-30: Replace ⏎············href='https://github.com/asyncapi/website/issues/new?template=blog.md'⏎············target='_blank'⏎··········
with ·href='https://github.com/asyncapi/website/issues/new?template=blog.md'·target='_blank'
prettier/prettier
[error] 41-41: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 66-66: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 76-76: Replace ⏎··············href='https://twitter.com/AsyncAPISpec'⏎··············className='mt-4·inline-flex·items-center'⏎··············target='_blank'⏎············
with ·href='https://twitter.com/AsyncAPISpec'·className='mt-4·inline-flex·items-center'·target='_blank'
prettier/prettier
[error] 102-102: Replace ⏎············level={HeadingLevel.h4}⏎············typeStyle={HeadingTypeStyle.mdSemibold}⏎··········
with ·level={HeadingLevel.h4}·typeStyle={HeadingTypeStyle.mdSemibold}
prettier/prettier
[error] 109-109: Delete ⏎···········
prettier/prettier
[error] 113-113: Replace ⏎··············href='https://www.youtube.com/c/AsyncAPI'⏎··············className='mt-4'⏎··············target='_blank'⏎············
with ·href='https://www.youtube.com/c/AsyncAPI'·className='mt-4'·target='_blank'
prettier/prettier
🔇 Additional comments (1)
components/newsroom/Newsroom.tsx (1)
87-95
: Good implementation of the card-based layout for articles.This change successfully implements a more flexible layout container for the news articles with proper responsive behavior and height constraints. The
max-h-96
class limits the height appropriately for better UI consistency.This layout change looks good, but I'd recommend verifying how it behaves with different amounts of content. The
max-h-96
(24rem/384px) might be restrictive if there are many articles. Does this container provide scrolling for overflow content as intended?
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/newsroom/Newsroom.tsx
(2 hunks)
🧰 Additional context used
🪛 ESLint
components/newsroom/Newsroom.tsx
[error] 25-25: Delete ··
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/newsroom/Newsroom.tsx
[error] 25-25: Delete ··
prettier/prettier
🔇 Additional comments (3)
components/newsroom/Newsroom.tsx (3)
64-66
: Improved Twitter follow button alignment.The addition of
inline-flex items-center
class and changing frominline
toml-2
improves the alignment of the Twitter follow button with proper spacing - which directly addresses the PR objective of enhancing the UI.
71-72
: Well-implemented card-based layout.The changes to the container div implement a responsive card-based layout with appropriate flex properties and overflow handling. This directly addresses the PR objective for redesigning the "Latest News" section with a card-based layout.
flex-row items-stretch justify-between
ensures proper alignment and spacingmd:flex
provides responsiveness for medium screens and upmax-h-96
withoverflow-y-auto
creates a scrollable container with fixed height
74-74
: Improved sizing for article container.The
size-full
class (shorthand for w-full and h-full) ensures the NewsroomArticle component takes up the full available space within its container, improving the layout.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3982 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 778
Branches 144 144
=========================================
Hits 778 778 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
/rtm |
@PankajKumardev LGTM |
Than, this PR is going to be merge? |
components/newsroom/Newsroom.tsx
Outdated
@@ -61,17 +61,17 @@ export default function Newsroom() { | |||
Read about what people are <br /> saying about AsyncAPI | |||
</Paragraph> | |||
<div className='my-5' data-testid='Newsroom-Twitter-Link'> | |||
<TextLink href='https://twitter.com/AsyncAPISpec' className='mt-4' target='_blank'> | |||
<TextLink href='https://twitter.com/AsyncAPISpec' className='mt-4 inline-flex items-center' target='_blank'> |
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.
is there any other alternative of using inline-flex
@Mayaleeeee this is a design change so can you please review this |
Is this not part of the GSoC project? |
it is minor fix so we can merge this one |
Hey bro, After the changes in AnnouncementHero: |
Just wanted to follow up on the PR I updated as per your suggestions. Let me know if there’s anything else you’d like me to address or if it's good to go. Appreciate your time and feedback! |
) * improve UI responsiveness for 'Latest News' section in Community Newsroom * fix the preetier part * refactor: standardize quotation marks in Newsroom component * liniting fix * lint fix * lint fix * fix: correct indentation in Newsroom component * Remove duplicate tool rendering in ToolsList using Set for unique titles. * Remove duplicate tool rendering in ToolsList using Set for unique titles. * Refactor ToolsList component to simplify rendering and remove duplicate tool handling * removed inline * fix: correct typo in Twitter link class name * fix: adjust width of banner container for better responsiveness --------- Co-authored-by: Akshat Nema <[email protected]>
Description
Before

After

Related issue(s)
Fixes #3587
Notes
Summary by CodeRabbit