-
Notifications
You must be signed in to change notification settings - Fork 505
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
feat(landing): add glossary #2085
Conversation
|
@p6l-richard is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
Warning Rate limit exceeded@p6l-richard has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduced a comprehensive glossary feature within the application, including a new glossary term page, client interface, and various supporting components. Key functionalities include dynamic term navigation, metadata generation, and a structured FAQ section. New UI components such as badges, buttons, cards, and input fields were added for enhanced user interaction. Additionally, a new collection for glossary terms was created, and several MDX documents were introduced to provide content on specific topics like MIME types. Changes
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
Thank you for following the naming conventions for pull request titles! 🙏 |
- UI: Add `faq`, `takeaway` & `relatedTerms` components (_faq.tsx, takeaways.tsx, page.tsx) - shadcn: Added new UI components (badge.tsx, card.tsx) - Updated content collections (content-collections.ts) for more frontmatter schema on glossary (reviewed by, takeaways, faq, etc.) - Add glossary content (check `mime-types.mdx`) for mocking - Update related configurations and utilities
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
… into richard/glossary
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 8
🧹 Outside diff range and nitpick comments (7)
apps/www/content/glossary/mime-types.mdx (3)
2-3
: Enhance the description for better SEO.Consider adding more specific keywords and making the description more actionable.
-description: "Learn about MIME types in API development. Understand their role in defining file formats like images and PDFs. Explore our glossary for more insights." +description: "Master MIME types in REST API development. Learn how to correctly use Content-Type headers, handle file formats, and implement secure content negotiation. Essential guide for web developers."
55-63
: Enhance HTTP header examples with complete response context.The HTTP header examples could be more educational by showing complete response contexts.
-```http -Content-Type: application/json -``` -This header in an API response indicates that the data is in JSON format. - -```http -Content-Type: text/html -``` +```http +HTTP/1.1 200 OK +Content-Type: application/json +Content-Length: 234 + +{ + "message": "Example JSON response" +} +``` +This complete API response shows the JSON Content-Type in context. + +```http +HTTP/1.1 200 OK +Content-Type: text/html; charset=UTF-8 +Content-Length: 156 + +<!DOCTYPE html> +<html> + <head><title>Example</title></head> + <body><h1>Hello World!</h1></body> +</html> +```
98-98
: Format URL as markdown link.Convert the plain text URL into a proper markdown link for better readability and accessibility.
-For further exploration, developers can refer to the official IANA Media Types registry (https://www.iana.org/assignments/media-types/) or the resources listed in the recommended reading section above. +For further exploration, developers can refer to the [official IANA Media Types registry](https://www.iana.org/assignments/media-types/) or the resources listed in the recommended reading section above.apps/www/app/glossary/[slug]/takeaways.tsx (1)
16-37
: Add JSDoc documentation for the schemas.Consider adding JSDoc comments to document the purpose and structure of both schemas. This would improve maintainability and help other developers understand the expected data structure.
+/** + * Schema for individual items in the takeaways sections + * @property {string} key - The label or title of the item + * @property {string} value - The corresponding value or description + */ const itemSchema = z.object({ key: z.string(), value: z.string(), }); +/** + * Schema for the complete takeaways data structure + * @property {string} tldr - A brief summary + * @property {Array<itemSchema>} definitionAndStructure - Key definitions and structural elements + * @property {Array<itemSchema>} historicalContext - Historical timeline or context + * @property {Object} usageInAPIs - API usage information with tags and description + * @property {Array<string>} bestPractices - List of recommended practices + * @property {Array<{title: string, url: string}>} recommendedReading - External resources + * @property {string} didYouKnow - An interesting fact + */ export const takeawaysSchema = z.object({apps/www/app/glossary/[slug]/page.tsx (3)
42-42
: Consider using environment-based domain configurationThe OpenGraph URL contains a hardcoded domain "unkey.com". Consider using an environment variable to make it configurable across different environments (development, staging, production).
- url: `https://unkey.com/glossary/${term.slug}`, + url: `${process.env.NEXT_PUBLIC_DOMAIN}/glossary/${term.slug}`,
76-317
: Consider adding error boundariesThe component handles the "not found" case but lacks error boundaries for runtime errors. Consider wrapping the content with an error boundary to gracefully handle unexpected errors.
Example implementation:
import { ErrorBoundary } from '@/components/error-boundary'; // Inside the component return ( <ErrorBoundary fallback={<div>Something went wrong loading the glossary term.</div>}> {/* existing JSX */} </ErrorBoundary> );
241-247
: Enhance avatar accessibilityThe avatar's alt text could be more descriptive to improve accessibility.
- alt={author.name} + alt={`${author.name}'s profile picture`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- apps/www/app/glossary/[slug]/faq.tsx (1 hunks)
- apps/www/app/glossary/[slug]/page.tsx (1 hunks)
- apps/www/app/glossary/[slug]/takeaways.tsx (1 hunks)
- apps/www/app/glossary/client.tsx (1 hunks)
- apps/www/content/glossary/mime-types.mdx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/www/app/glossary/client.tsx
🧰 Additional context used
🪛 Biome
apps/www/app/glossary/[slug]/page.tsx
[error] 59-59: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend terms with an underscore.(lint/correctness/noUnusedVariables)
🪛 LanguageTool
apps/www/content/glossary/mime-types.mdx
[style] ~71-~71: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNGs”.
Context: ...egfor JPEG images -
image/pngfor PNG images -
image/gif` for GIF images - **Vid...(ACRONYM_TAUTOLOGY)
🔇 Additional comments (3)
apps/www/app/glossary/[slug]/faq.tsx (1)
1-6
: LGTM! Imports are well-organized.The necessary accordion components are properly imported from the UI library.
apps/www/content/glossary/mime-types.mdx (2)
69-73
: Enhance image type descriptions with full expansions.The previous review already identified this issue. I agree with expanding the acronyms for better clarity, while avoiding redundancy.
🧰 Tools
🪛 LanguageTool
[style] ~71-~71: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNGs”.
Context: ...egfor JPEG images -
image/pngfor PNG images -
image/gif` for GIF images - **Vid...(ACRONYM_TAUTOLOGY)
36-41
: Verify the accessibility of recommended reading URLs.The recommended reading section includes external URLs that should be validated to ensure they are accessible and up-to-date.
✅ Verification successful
All recommended reading URLs are accessible and valid
All three URLs in the recommended reading section return HTTP 200 status codes, indicating they are accessible and up-to-date:
- RFC 6838 specification ✓
- MDN Web Docs article ✓
- O'Reilly book reference ✓
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the recommended reading URLs are accessible # Expected results: All URLs should return HTTP 200 status urls=( "https://tools.ietf.org/html/rfc6838" "https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types" "https://www.oreilly.com/library/view/restful-web-services/9780596529260/" ) for url in "${urls[@]}"; do echo "Checking $url" curl -sL -w "%{http_code}\n" "$url" -o /dev/null doneLength of output: 1057
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/www/app/glossary/[slug]/page.tsx (1 hunks)
- apps/www/app/glossary/[slug]/takeaways.tsx (1 hunks)
🧰 Additional context used
🪛 Biome
apps/www/app/glossary/[slug]/page.tsx
[error] 59-59: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend terms with an underscore.(lint/correctness/noUnusedVariables)
🔇 Additional comments (1)
apps/www/app/glossary/[slug]/page.tsx (1)
59-74
: Remove unused terms array.The
terms
array is declared but never used in the code.🧰 Tools
🪛 Biome
[error] 59-59: This variable is unused.
Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
Unsafe fix: If this is intentional, prepend terms with an underscore.(lint/correctness/noUnusedVariables)
const GlossaryTermWrapper = async ({ params }: { params: { slug: string } }) => { | ||
const term = allGlossaries.find((term) => term.slug === params.slug); | ||
if (!term) { |
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
Consider using type assertion for better type safety.
The current implementation might return undefined if the term is not found. Consider using type assertion with find for better type safety.
Apply this diff:
- const term = allGlossaries.find((term) => term.slug === params.slug);
+ const term = allGlossaries.find((term) => term.slug === params.slug) ?? notFound();
📝 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.
const GlossaryTermWrapper = async ({ params }: { params: { slug: string } }) => { | |
const term = allGlossaries.find((term) => term.slug === params.slug); | |
if (!term) { | |
const GlossaryTermWrapper = async ({ params }: { params: { slug: string } }) => { | |
const term = allGlossaries.find((term) => term.slug === params.slug) ?? notFound(); | |
if (!term) { |
export function generateMetadata({ | ||
params, | ||
}: { | ||
params: { slug: string }; | ||
}): Metadata { | ||
const term = allGlossaries.find((term) => term.slug === params.slug); | ||
if (!term) { | ||
notFound(); | ||
} | ||
return { | ||
title: `${term.title} | Unkey Glossary`, | ||
description: term.description, | ||
openGraph: { | ||
title: `${term.title} | Unkey Glossary`, | ||
description: term.description, | ||
url: `https://unkey.com/glossary/${term.slug}`, | ||
siteName: "unkey.com", | ||
type: "article", | ||
}, | ||
twitter: { | ||
card: "summary_large_image", | ||
title: `${term.title} | Unkey Glossary`, | ||
description: term.description, | ||
site: "@unkeydev", | ||
creator: "@unkeydev", | ||
}, | ||
icons: { | ||
shortcut: "/images/landing/unkey.png", | ||
}, | ||
}; | ||
} |
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
Enhance metadata generation with safer URL handling and path validation.
Consider these improvements for better robustness:
- Use URL constructor for safer URL handling
- Validate the icons path exists
Apply this diff:
return {
title: `${term.title} | Unkey Glossary`,
description: term.description,
openGraph: {
title: `${term.title} | Unkey Glossary`,
description: term.description,
- url: `https://unkey.com/glossary/${term.slug}`,
+ url: new URL(`/glossary/${term.slug}`, 'https://unkey.com').toString(),
siteName: "unkey.com",
type: "article",
},
// ... rest of the metadata
};
📝 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.
export function generateMetadata({ | |
params, | |
}: { | |
params: { slug: string }; | |
}): Metadata { | |
const term = allGlossaries.find((term) => term.slug === params.slug); | |
if (!term) { | |
notFound(); | |
} | |
return { | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
openGraph: { | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
url: `https://unkey.com/glossary/${term.slug}`, | |
siteName: "unkey.com", | |
type: "article", | |
}, | |
twitter: { | |
card: "summary_large_image", | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
site: "@unkeydev", | |
creator: "@unkeydev", | |
}, | |
icons: { | |
shortcut: "/images/landing/unkey.png", | |
}, | |
}; | |
} | |
export function generateMetadata({ | |
params, | |
}: { | |
params: { slug: string }; | |
}): Metadata { | |
const term = allGlossaries.find((term) => term.slug === params.slug); | |
if (!term) { | |
notFound(); | |
} | |
return { | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
openGraph: { | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
url: new URL(`/glossary/${term.slug}`, 'https://unkey.com').toString(), | |
siteName: "unkey.com", | |
type: "article", | |
}, | |
twitter: { | |
card: "summary_large_image", | |
title: `${term.title} | Unkey Glossary`, | |
description: term.description, | |
site: "@unkeydev", | |
creator: "@unkeydev", | |
}, | |
icons: { | |
shortcut: "/images/landing/unkey.png", | |
}, | |
}; | |
} |
const itemSchema = z.object({ | ||
key: z.string(), | ||
value: z.string(), | ||
}); | ||
|
||
export const takeawaysSchema = z.object({ | ||
tldr: z.string(), | ||
definitionAndStructure: z.array(itemSchema), | ||
historicalContext: z.array(itemSchema), | ||
usageInAPIs: z.object({ | ||
tags: z.array(z.string()), | ||
description: z.string(), | ||
}), | ||
bestPractices: z.array(z.string()), | ||
recommendedReading: z.array( | ||
z.object({ | ||
title: z.string(), | ||
url: z.string(), | ||
}), | ||
), | ||
didYouKnow: z.string(), | ||
}); |
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
Utilize takeawaysSchema
for runtime validation of props
While you've defined itemSchema
and takeawaysSchema
using zod
for schema validation, the current implementation doesn't use them for runtime validation of props.takeaways
. Consider parsing props.takeaways
with takeawaysSchema.parse()
to ensure the data conforms to the expected structure, which can help catch invalid data early and prevent potential runtime errors.
You can update the Takeaways
component as follows:
export default function Takeaways(props: Pick<Glossary, "term" | "takeaways">) {
+ const takeaways = takeawaysSchema.parse(props.takeaways);
return (
- <Card> {/* ...using props.takeaways... */}</Card>
+ <Card> {/* ...use takeaways instead of props.takeaways... */}</Card>
);
}
Committable suggestion was skipped due to low confidence.
What does this PR do?
Demo 👇
https://33fa1ur95-b7flgfdfq-unkey.vercel.app/glossary
https://33fa1ur95-b7flgfdfq-unkey.vercel.app/glossary/mime-types
Adding a
/glossary
to unkey's landing.Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
Release Notes
New Features
GlossaryTermWrapper
for detailed term pages.GlossaryClient
for an interactive glossary interface, featuring search and category navigation.TermsRolodexDesktop
) and mobile (TermsStepperMobile
) views for glossary term navigation.Documentation
UI Enhancements
Badge
,Button
,Card
,Input
,Label
, andKeyIcon
for improved styling and user interaction.