-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: #4065 Case Studies page table is now mobile responsive #4283
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. |
WalkthroughThis update modifies the Case Studies page to improve table styling and accessibility, including attribute formatting, class updates, and removal of deprecated HTML. Additionally, a documentation comment URL is updated in a TypeScript environment file. No functional or logic changes are introduced. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4283--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: 2
🧹 Nitpick comments (2)
pages/casestudies/index.tsx (2)
110-113: Hard-codedmin-w-[600px]still forces horizontal scroll on narrow phonesThe new container adds
overflow-x-auto, but the table itself is fixed at a minimum width of 600 px.
On devices ≤360 px this still overflows and requires scrolling.Consider a fully responsive pattern—e.g. convert the table into stacked cards under
mdbreakpoint or remove the rigidmin-w:-<table className="my-8 min-w-[600px] w-full border-collapse border"> +<table className="my-8 w-full border-collapse border md:min-w-[600px]">(or use CSS grid / flexbox representation for small screens).
115-120: Add semantic scope for table headersTo improve accessibility for screen-reader users, add
scope="col"on each<th>:-<th className='border p-2 font-bold text-left'>Company name</th> +<th scope="col" className='border p-2 font-bold text-left'>Company name</th>Repeat for the other two headers.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
next-env.d.ts(1 hunks)pages/casestudies/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
next-env.d.ts (5)
Learnt from: anshgoyalevil
PR: #3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., require('../scripts/build-rss.ts')). This is enforced by the project's configuration which uses moduleResolution: "bundler" in tsconfig.json and TypeScript-aware ESLint plugins. The .ts extensions are required even though the files are imported using CommonJS require statements.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., require('../scripts/build-rss.ts')). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: anshgoyalevil
PR: #3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in tests/fixtures/markdown/check-edit-links-data.js intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: akshatnema
PR: #3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In JavaScript test files within the AsyncAPI website project, TypeScript file imports must include the .ts extension to avoid lint errors, even though the files being imported are JavaScript files.
pages/casestudies/index.tsx (3)
Learnt from: akshatnema
PR: #3262
File: components/Avatar.tsx:35-44
Timestamp: 2024-10-11T10:46:58.882Z
Learning: In Next.js, when avoiding nested <a> tags due to hydration issues, use accessible non-link elements like <button> or <span> with appropriate roles, tabIndex, and event handlers to maintain accessibility and SEO.
Learnt from: akshatnema
PR: #3262
File: components/newsroom/FeaturedBlogPost.tsx:90-92
Timestamp: 2024-10-11T07:38:35.745Z
Learning: In Next.js, nested <a> tags cause hydration issues due to invalid HTML. To fix this, avoid nesting <a> tags by replacing inner <a> tags with non-interactive elements like <span>, and use click handlers to maintain interactivity if needed.
Learnt from: akshatnema
PR: #3262
File: components/navigation/BlogPostItem.tsx:95-119
Timestamp: 2024-10-11T11:32:30.226Z
Learning: In the BlogPostItem component (components/navigation/BlogPostItem.tsx), nesting <a> tags inside the parent Link component leads to hydration issues; therefore, we should avoid nesting <a> tags inside Link components in this component.
🪛 GitHub Actions: PR testing - if Node project
pages/casestudies/index.tsx
[error] 30-141: Prettier formatting errors and jsx-quotes linting errors detected. Multiple instances of double quotes used instead of single quotes and unnecessary line breaks. Run 'prettier --write' to fix code style issues.
🔇 Additional comments (1)
next-env.d.ts (1)
5-5: Comment-only change looks goodThe comment URL update is correct and has no impact on typings. Nothing further to do here.
| className="relative mx-auto max-w-xl px-4 py-10 sm:px-6 lg:max-w-screen-xl lg:px-8" | ||
| data-testid="CaseStudy-main" | ||
| > | ||
| <div className='grid lg:grid-cols-9 lg:gap-8 lg:text-center'> | ||
| <div className='col-span-5 col-start-3'> | ||
| <div className="grid lg:grid-cols-9 lg:gap-8 lg:text-center"> | ||
| <div className="col-span-5 col-start-3"> | ||
| <Heading level={HeadingLevel.h1} typeStyle={HeadingTypeStyle.lg}> | ||
| {title} | ||
| </Heading> | ||
| <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'> | ||
| The best way to learn how to use AsyncAPI is not only through documentation that usually is focused on | ||
| recommendations and best practices. It is also good to confront with real-life case studies that explain | ||
| how people really use AsyncAPI and what are their flows. | ||
| <Paragraph | ||
| typeStyle={ParagraphTypeStyle.md} | ||
| className="my-4 max-w-4xl" | ||
| > | ||
| The best way to learn how to use AsyncAPI is not only through | ||
| documentation that usually is focused on recommendations and best | ||
| practices. It is also good to confront with real-life case studies | ||
| that explain how people really use AsyncAPI and what are their | ||
| flows. | ||
| </Paragraph> | ||
| <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'> | ||
| Feel free to submit your case study. We have a template for you. For more details | ||
| <TextLink href='https://github.com/asyncapi/website/blob/master/README.md#case-studies' target='_blank'> | ||
| <Paragraph | ||
| typeStyle={ParagraphTypeStyle.md} | ||
| className="my-4 max-w-4xl" | ||
| > | ||
| Feel free to submit your case study. We have a template for you. | ||
| For more details | ||
| <TextLink | ||
| href="https://github.com/asyncapi/website/blob/master/README.md#case-studies" | ||
| target="_blank" | ||
| > | ||
| read our FAQ | ||
| </TextLink> | ||
| . | ||
| </Paragraph> | ||
| </div> | ||
| </div> | ||
| <div data-testid='CaseStudy-card'> | ||
| <div data-testid="CaseStudy-card"> | ||
| <CaseStudyCard studies={CaseStudiesList as ICaseStudies} /> | ||
| </div> | ||
|
|
||
| <div className='adopters'> | ||
| <div className='mt-8 grid lg:grid-cols-9 lg:gap-8 lg:text-center'> | ||
| <div className='col-span-5 col-start-3'> | ||
| <div className="adopters"> | ||
| <div className="mt-8 grid lg:grid-cols-9 lg:gap-8 lg:text-center"> | ||
| <div className="col-span-5 col-start-3"> | ||
| <Heading level={HeadingLevel.h1} typeStyle={HeadingTypeStyle.lg}> | ||
| Adopters | ||
| </Heading> | ||
| <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'> | ||
| Check out how different companies use AsyncAPI and what problems they solve. | ||
| <Paragraph | ||
| typeStyle={ParagraphTypeStyle.md} | ||
| className="my-4 max-w-4xl" | ||
| > | ||
| Check out how different companies use AsyncAPI and what problems | ||
| they solve. | ||
| </Paragraph> | ||
| <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'> | ||
| <Paragraph | ||
| typeStyle={ParagraphTypeStyle.md} | ||
| className="my-4 max-w-4xl" | ||
| > | ||
| Feel free to{' '} | ||
| <a className='underline' href='https://github.com/asyncapi/website/blob/master/config/adopters.yml'> | ||
| <a | ||
| className="underline" | ||
| href="https://github.com/asyncapi/website/blob/master/config/adopters.yml" | ||
| > | ||
| submit a pull request | ||
| </a>{' '} | ||
| with information about how your company uses AsyncAPI. We know that writing an official case study might | ||
| be time consuming and requires too much internal paper work. Let's make sure we can at least | ||
| capture a use case that is already a great learning information for the community. | ||
| with information about how your company uses AsyncAPI. We know | ||
| that writing an official case study might be time consuming and | ||
| requires too much internal paper work. Let's make sure we | ||
| can at least capture a use case that is already a great learning | ||
| information for the community. | ||
| </Paragraph> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <center> | ||
| <div className='overflow-x-auto'> | ||
| <table className='my-8 w-full max-w-full border-collapse border'> | ||
| <div className="w-full flex justify-center"> | ||
| <div className="overflow-x-auto w-full"> | ||
| <table className="my-8 min-w-[600px] w-full border-collapse border"> | ||
| <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 className="border p-2 font-bold text-left"> | ||
| Company name | ||
| </th> | ||
| <th className="border p-2 font-bold text-left">Use Case</th> | ||
| <th className="border p-2 font-bold text-left">Resources</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {AdoptersList.map((entry: Adopter, index: number) => ( | ||
| <tr key={index} 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> | ||
| {entry.resources.map((resource: Resource, resourceIndex: number) => ( | ||
| <li className='p-2' key={resourceIndex}> | ||
| <a className='text-cyan-500 underline' href={resource.link}> | ||
| {resource.title} | ||
| </a> | ||
| </li> | ||
| ))} | ||
| <tr key={index} data-testid="Adopters"> | ||
| <td className="border p-2">{entry.companyName}</td> | ||
| <td className="border p-2">{entry.useCase}</td> | ||
| <td className="border p-2"> | ||
| <ul className="list-disc pl-4"> | ||
| {entry.resources.map( | ||
| (resource: Resource, resourceIndex: number) => ( | ||
| <li key={resourceIndex} className="py-1"> | ||
| <a | ||
| className="text-cyan-500 underline" | ||
| href={resource.link} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| {resource.title} | ||
| </a> | ||
| </li> | ||
| ), | ||
| )} | ||
| </ul> | ||
| </td> | ||
| </tr> | ||
| ))} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| </center> | ||
| </div> |
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.
Formatting breaks project style ‑ fix Prettier / jsx-quotes violations
All newly-added JSX attributes were switched to double quotes and extra line-breaks.
The repo ESLint/Prettier config (jsx-quotes: "prefer-single") expects single quotes and no manual wrapping.
CI is already red (Prettier formatting errors and jsx-quotes linting errors detected).
Run the project formatter to revert to canonical style or explicitly use single quotes when writing JSX:
-<div className="relative mx-auto max-w-xl px-4 py-10 sm:px-6 lg:max-w-screen-xl lg:px-8">
+<div className='relative mx-auto max-w-xl px-4 py-10 sm:px-6 lg:max-w-screen-xl lg:px-8'>…and so on for every JSX literal introduced in this PR.
After fixing the quotes, execute:
pnpm prettier --write .
pnpm eslint --fix .to ensure the file passes the pipeline.
🤖 Prompt for AI Agents
In pages/casestudies/index.tsx from lines 38 to 150, the JSX attributes use
double quotes and have extra line breaks, which violates the project's Prettier
and eslint jsx-quotes rule that prefers single quotes and no manual wrapping. To
fix this, replace all double quotes in JSX attributes with single quotes and
remove unnecessary line breaks. Then run the project formatter with pnpm
prettier --write . and pnpm eslint --fix . to ensure the file conforms to the
project's style and passes the CI checks.
| {AdoptersList.map((entry: Adopter, index: number) => ( | ||
| <tr key={index} 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> | ||
| {entry.resources.map((resource: Resource, resourceIndex: number) => ( | ||
| <li className='p-2' key={resourceIndex}> | ||
| <a className='text-cyan-500 underline' href={resource.link}> | ||
| {resource.title} | ||
| </a> | ||
| </li> | ||
| ))} | ||
| <tr key={index} data-testid="Adopters"> | ||
| <td className="border p-2">{entry.companyName}</td> | ||
| <td className="border p-2">{entry.useCase}</td> | ||
| <td className="border p-2"> | ||
| <ul className="list-disc pl-4"> | ||
| {entry.resources.map( | ||
| (resource: Resource, resourceIndex: number) => ( | ||
| <li key={resourceIndex} className="py-1"> | ||
| <a | ||
| className="text-cyan-500 underline" | ||
| href={resource.link} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| {resource.title} | ||
| </a> | ||
| </li> | ||
| ), | ||
| )} | ||
| </ul> | ||
| </td> | ||
| </tr> | ||
| ))} |
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.
🛠️ Refactor suggestion
Avoid using array indices as React keys
<tr key={index}> and <li key={resourceIndex}> rely on positional indices.
If the underlying lists ever re-order, React will mis-match DOM nodes, causing subtle UI bugs.
Use a stable, unique identifier instead (company name, resource link):
-<tr key={index} data-testid="Adopters">
+<tr key={entry.companyName} data-testid="Adopters">
...
- <li key={resourceIndex} className="py-1">
+ <li key={resource.link} className="py-1">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {AdoptersList.map((entry: Adopter, index: number) => ( | |
| <tr key={index} 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> | |
| {entry.resources.map((resource: Resource, resourceIndex: number) => ( | |
| <li className='p-2' key={resourceIndex}> | |
| <a className='text-cyan-500 underline' href={resource.link}> | |
| {resource.title} | |
| </a> | |
| </li> | |
| ))} | |
| <tr key={index} data-testid="Adopters"> | |
| <td className="border p-2">{entry.companyName}</td> | |
| <td className="border p-2">{entry.useCase}</td> | |
| <td className="border p-2"> | |
| <ul className="list-disc pl-4"> | |
| {entry.resources.map( | |
| (resource: Resource, resourceIndex: number) => ( | |
| <li key={resourceIndex} className="py-1"> | |
| <a | |
| className="text-cyan-500 underline" | |
| href={resource.link} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| {resource.title} | |
| </a> | |
| </li> | |
| ), | |
| )} | |
| </ul> | |
| </td> | |
| </tr> | |
| ))} | |
| {AdoptersList.map((entry: Adopter, index: number) => ( | |
| <tr key={entry.companyName} data-testid="Adopters"> | |
| <td className="border p-2">{entry.companyName}</td> | |
| <td className="border p-2">{entry.useCase}</td> | |
| <td className="border p-2"> | |
| <ul className="list-disc pl-4"> | |
| {entry.resources.map( | |
| (resource: Resource, resourceIndex: number) => ( | |
| <li key={resource.link} className="py-1"> | |
| <a | |
| className="text-cyan-500 underline" | |
| href={resource.link} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| {resource.title} | |
| </a> | |
| </li> | |
| ), | |
| )} | |
| </ul> | |
| </td> | |
| </tr> | |
| ))} |
🤖 Prompt for AI Agents
In pages/casestudies/index.tsx between lines 123 and 146, replace the array
index keys used in the <tr> and <li> elements with stable, unique identifiers.
Use entry.companyName as the key for the <tr> elements and resource.link as the
key for the <li> elements to ensure React can correctly track list items even if
the order changes.
fix: #4065
Description
fix: #4065
Summary by CodeRabbit
Style
Accessibility
Documentation