-
-
Notifications
You must be signed in to change notification settings - Fork 981
fix: #4065 Made Case Studies page table section mobile responsive #4403
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
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReworked two pages: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4403 +/- ##
=========================================
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:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4403--asyncapi-website.netlify.app/ |
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: 0
🧹 Nitpick comments (5)
pages/casestudies/index.tsx (5)
87-95: Add caption + scope for table a11y.Improve screen reader semantics with a caption and scope="col" on headers.
Apply:
- <table className='my-8 w-full border-collapse border table-auto'> + <table className='my-8 w-full border-collapse border table-auto' aria-label='Adopters'> + <caption className='sr-only'>Adopters: company, use case, and resources</caption> <thead> <tr> - <th className='border p-2 font-bold'>Company name</th> - <th className='border-2 p-2 font-bold'>Use Case</th> - <th className='border-2 p-2 font-bold'>Resources</th> + <th scope='col' className='border-2 p-2 font-bold'>Company Name</th> + <th scope='col' className='border-2 p-2 font-bold'>Use Case</th> + <th scope='col' className='border-2 p-2 font-bold'>Resources</th>
96-110: Use stable keys, standardize link component, and handle empty resources.Avoid index keys, reuse TextLink, and render a fallback when resources are empty.
Apply:
- {AdoptersList.map((entry: Adopter, index: number) => ( - <tr key={index} data-testid='Adopters'> + {AdoptersList.map((entry: Adopter) => ( + <tr key={entry.companyName} data-testid='Adopters'> <td className='border-2 p-2'>{entry.companyName}</td> <td className='border-2 p-2'>{entry.useCase}</td> <td className='border-2 p-2'> - <ul className='space-y-1'> - {entry.resources.map((resource: Resource, resourceIndex: number) => ( - <li key={resourceIndex}> - <a className='text-cyan-500 underline hover:text-cyan-600' href={resource.link}> - {resource.title} - </a> - </li> - ))} - </ul> + {entry.resources?.length ? ( + <ul className='space-y-1'> + {entry.resources.map((resource: Resource) => ( + <li key={resource.link}> + <TextLink + href={resource.link} + target='_blank' + rel='noopener noreferrer' + className='text-cyan-500 underline hover:text-cyan-600' + > + {resource.title} + </TextLink> + </li> + ))} + </ul> + ) : ( + <span className='text-gray-500'>—</span> + )} </td> </tr> ))}
90-92: Minor consistency nits: border thickness and capitalization.
- Use the same border class across all headers/cells.
- Capitalize “Company Name” to match other headers.
Example:
-<th className='border p-2 font-bold'>Company name</th> +<th scope='col' className='border-2 p-2 font-bold'>Company Name</th>Also applies to: 98-100
121-140: Stable keys, dark mode, and touch-friendly links on mobile.
- Prefer stable keys.
- Add dark-mode styles to avoid bright cards in dark theme.
- Make links block-level with padding for better tap targets; reuse TextLink with target+rel.
Apply:
- <div - key={index} - className='border border-gray-300 rounded-lg p-4 bg-white shadow-sm' + <div + key={entry.companyName} + className='border border-gray-300 dark:border-gray-700 rounded-lg p-4 bg-white dark:bg-slate-900 shadow-sm' data-testid='Adopters' > @@ - <h3 className='font-bold text-lg text-gray-900'>{entry.companyName}</h3> + <h3 className='font-bold text-lg text-gray-900 dark:text-gray-100'>{entry.companyName}</h3> @@ - <h4 className='font-semibold text-sm text-gray-700 mb-1'>Use Case:</h4> - <p className='text-gray-600 text-sm'>{entry.useCase}</p> + <h4 className='font-semibold text-sm text-gray-700 dark:text-gray-300 mb-1'>Use Case:</h4> + <p className='text-gray-600 dark:text-gray-300 text-sm'>{entry.useCase}</p> @@ - <h4 className='font-semibold text-sm text-gray-700 mb-2'>Resources:</h4> + <h4 className='font-semibold text-sm text-gray-700 dark:text-gray-300 mb-2'>Resources:</h4> <ul className='space-y-1'> - {entry.resources.map((resource: Resource, resourceIndex: number) => ( - <li key={resourceIndex}> - <a className='text-cyan-500 underline hover:text-cyan-600 text-sm' href={resource.link}> - {resource.title} - </a> - </li> - ))} + {entry.resources?.length ? ( + entry.resources.map((resource: Resource) => ( + <li key={resource.link}> + <TextLink + href={resource.link} + target='_blank' + rel='noopener noreferrer' + className='text-cyan-500 underline hover:text-cyan-600 text-sm block py-1.5' + > + {resource.title} + </TextLink> + </li> + )) + ) : ( + <li className='text-gray-500 text-sm'>—</li> + )}
96-110: DRY: consider extracting AdopterTableRow and AdopterCard components.Reduces duplication between desktop and mobile render paths and keeps keys/link handling in one place.
Also applies to: 119-146
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/casestudies/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
🔇 Additional comments (4)
pages/casestudies/index.tsx (4)
85-116: Clean responsive split (desktop table + mobile cards).Layout gating with md:hidden/md:block is clear and keeps markup simple. Nice work.
117-146: Mobile cards: good structure and spacing.Card breakdown reads well and aligns with the PR goal.
97-97: Duplicate data-testid across views may confuse tests.Both views use data-testid="Adopters". Ensure tests query by visible container or refine testids per view (e.g., AdoptersRow vs AdoptersCard).
Also applies to: 124-124
74-76: Verify contributors link matches source format.The page links to config/adopters.yml while this file imports config/adopters.json. Confirm the canonical contribution path.
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: 0
🧹 Nitpick comments (2)
pages/tools/generator.tsx (2)
21-28: Use i18n key and correct GitHub casing; prefer canonical URLGithubButton expects a translation key (it calls t(text)). Passing a literal “View on Github” risks missing translations and uses incorrect casing. Also switch to the canonical github.com URL.
Apply:
- <GithubButton - text='View on Github' - className='w-full text-center md:w-auto md:text-left' - href='https://www.github.com/asyncapi/generator' - /> + <GithubButton + /* use i18n key; or omit `text` to fall back to the default 'githubButton' */ + text='githubButton' + className='w-full text-center md:w-auto md:text-left' + href='https://github.com/asyncapi/generator' + />If you prefer explicit keys, confirm the presence of something like common.viewOnGitHub and use that instead.
27-27: Add a test id (and consider i18n) for the Docs buttonHelps E2E tests target this CTA; optionally localize its label for consistency.
- <Button text='View Docs' href='/docs/tools/generator' className='w-full text-center md:w-auto md:text-left' /> + <Button + text='View Docs' + href='/docs/tools/generator' + className='w-full text-center md:w-auto md:text-left' + data-testid='Docs-button' + />If you want i18n: move this into a small component that uses useTranslation('common') and pass text={t('viewDocs')} (keeping GithubButton on a key, not a translated string).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pages/tools/generator.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pages/tools/generator.tsx (2)
components/buttons/GithubButton.tsx (1)
GithubButton(22-45)components/buttons/Button.tsx (1)
Button(53-106)
🔇 Additional comments (1)
pages/tools/generator.tsx (1)
21-21: Responsive container LGTMColumn on mobile → row on md+ with sensible wrap/justification. Meets the PR’s responsiveness goal.
|
I seriously love the design |
| <h4 className='font-semibold text-sm text-gray-700 mb-2'>Resources:</h4> | ||
| <ul className='space-y-1'> | ||
| {entry.resources.map((resource: Resource, resourceIndex: number) => ( | ||
| <li key={resourceIndex}> |
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.
what are the things we are mapping in resources
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.
-
Each adopter has a list of “resources” in config/adopters.json.
-
Every resource has two fields:
- title: what we show as the link text (e.g., “Blog post”, “GitHub repo”)
- link: the URL the user goes to when they click
- On the Case Studies page, we loop through "entry.resources" and render each one as a link.
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {AdoptersList.map((entry: Adopter, 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.
what does number signify here
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.
index: number explicitly documents that the map callback’s second argument is a 0-based numeric index, improving readability and type safety (even though TypeScript could infer it).
sambhavgupta0705
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.
LGTM
* docs(community): update latest community documentation (#4407) * chore: update meetings.json and newsrooom_videos.json (#4410) * chore: update meetings.json and newsrooom_videos.json (#4405) * chore: update meetings.json, newsrooom_videos.json and dashboard.json * Update meetings.json --------- Co-authored-by: asyncapi-bot <[email protected]> Co-authored-by: Eve <[email protected]> Co-authored-by: Sambhav Gupta <[email protected]> * chore: update meetings.json and newsrooom_videos.json (#4411) * fix: #4065 Made Case Studies page table section mobile responsive (#4403) * fixes #4065. Made Case Studie page mobile responsive * fixes #4065. Made Case Studie page mobile responsive * fix: #4381 --------- Co-authored-by: Sambhav Gupta <[email protected]> * chore(deps): bump axios from 1.8.2 to 1.12.1 (#4413) * chore(deps): bump @babel/runtime and next-language-detector (#4414) * chore: update tools.json (#4417) * chore: update meetings.json and newsrooom_videos.json (#4419) * ci: update of files from global .github repo (#4418) * fix: broken AsyncAPI contributing guidelines link (#4395) (#4398) Co-authored-by: SanidhyaMadheshia <[email protected]> Co-authored-by: Sambhav Gupta <[email protected]> Co-authored-by: V Thulisile Sibanda <[email protected]> --------- Co-authored-by: Chan <[email protected]> Co-authored-by: asyncapi-bot <[email protected]> Co-authored-by: Eve <[email protected]> Co-authored-by: namanjain24-sudo <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SanidhyaMadheshia <[email protected]> Co-authored-by: SanidhyaMadheshia <[email protected]> Co-authored-by: V Thulisile Sibanda <[email protected]>
Description
2.Mobile Card Structure:
Related issue(s)
#4065 : Case Studies page table is not mobile responsive
Summary by CodeRabbit
New Features
Refactor
Style