Conversation
|
@Theiaz, Love it so far, let me know when the descriptions are in. No issue with the headings; they're fine to include, but it's hard to tell without descriptions. Just tag me again when you want another look, and thanks again for contributing. |
7c6d35e to
3b53cb8
Compare
3b53cb8 to
4310574
Compare
|
@AlexanderKaran The first version with a short description of all the patterns is ready for review. Not sure whiter we need to include all of them. I think with future iteration we can refine them. I also asked myself if it's better to provide the glossary content inside a markdown file instead of directly editing the html. What do you think? Also UX for the glossary can be improved by providing a ToC in a sidebar, but i think this should be another issue. |
There was a problem hiding this comment.
Pull request overview
This PR adds a glossary page to the Framework Tracker documentation site to clarify terminology around web application architectures (MPA/SPA) and rendering patterns (SSR, SSG, CSR, hydration techniques, etc.). This addresses issue #134 which identified confusion around these terms throughout the project. The PR also extracts a reusable BackLink component that was previously duplicated in FrameworkDetail.
Changes:
- Added a comprehensive glossary page covering application architectures and rendering patterns
- Extracted BackLink component to improve code reusability
- Updated FrameworkDetail component to use the new BackLink component
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| packages/docs/src/pages/glossary.astro | New glossary page with detailed definitions for MPA, SPA, SSR, SSG, CSR, hydration variants, ISR, PPR, streaming, RSC, and ESR |
| packages/docs/src/components/BackLink.astro | Extracted reusable back link component with consistent styling |
| packages/docs/src/components/FrameworkDetail.astro | Refactored to use the new BackLink component, removed duplicate styles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <header> | ||
| <h1 class="detail-title">Glossary</h1> | ||
| <p> | ||
| This site describes the terminology and concepts used in the framework | ||
| tracker. | ||
| </p> | ||
| </header> |
There was a problem hiding this comment.
The glossary page lacks a way to navigate to it from the main site. There's no link in the navigation bar or on the home page. Users won't be able to discover this page unless they manually type the URL. Consider adding a link in the Navbar component or creating a prominent link on the home page.
| section { | ||
| width: 100%; | ||
| max-width: 900px; | ||
| } | ||
|
|
||
| @media screen and (max-width: 768px) { | ||
| main { | ||
| padding: 20px 16px; | ||
| } | ||
| } | ||
|
|
||
| .detail-header { | ||
| margin-bottom: 2em; | ||
| } | ||
|
|
||
| .detail-title { | ||
| font-size: 32px; | ||
| margin: 0 0 0.25em 0; | ||
| font-weight: 700; | ||
| background: var(--ft-gradient); | ||
| -webkit-background-clip: text; | ||
| -webkit-text-fill-color: transparent; | ||
| background-clip: text; | ||
| } | ||
|
|
||
| p, | ||
| ul { | ||
| font-size: 16px; | ||
| color: var(--ft-muted); | ||
| line-height: 1.6; | ||
| margin-bottom: 1em; | ||
| max-width: 700px; | ||
| } | ||
|
|
||
| section { | ||
| margin-top: 2em; | ||
| } |
There was a problem hiding this comment.
The selector 'section' is defined twice (lines 272-275 and 306-308). These should be merged into a single rule block to avoid confusion and potential styling conflicts. The second definition will override any conflicting properties from the first.
| h1 { | ||
| font-size: 32px; | ||
| margin-top: 0.25em; | ||
| margin-bottom: 0.5em; | ||
| font-weight: 700; | ||
| background: var(--ft-gradient); | ||
| -webkit-background-clip: text; | ||
| -webkit-text-fill-color: transparent; | ||
| background-clip: text; | ||
| } | ||
|
|
There was a problem hiding this comment.
The h1 styles (lines 310-319) duplicate the .detail-title styles (lines 287-295). Since the h1 element already has the class="detail-title" (line 11), the h1 selector is redundant. Consider removing the h1 rule block to avoid duplication.
| h1 { | |
| font-size: 32px; | |
| margin-top: 0.25em; | |
| margin-bottom: 0.5em; | |
| font-weight: 700; | |
| background: var(--ft-gradient); | |
| -webkit-background-clip: text; | |
| -webkit-text-fill-color: transparent; | |
| background-clip: text; | |
| } |
| .detail-header { | ||
| margin-bottom: 2em; | ||
| } |
There was a problem hiding this comment.
The .detail-header class is defined in the styles but never used in the HTML markup. The header element on line 10 doesn't have this class applied. Either add the class to the header element or remove the unused style rule.
| import Layout from '../layouts/Layout.astro' | ||
| --- | ||
|
|
||
| <Layout title="Glossary page"> |
There was a problem hiding this comment.
The page title should follow the pattern used by other pages in the codebase. The framework detail pages use the format "Page Name — Framework Tracker". Consider changing the title from "Glossary page" to "Glossary — Framework Tracker" for consistency.
| <Layout title="Glossary page"> | |
| <Layout title="Glossary — Framework Tracker"> |
| </p> | ||
| <p> | ||
| The term SSR is often used together with | ||
| <a href="#hydration">hydration</a>. However, classic SSR works without |
There was a problem hiding this comment.
The term "SSR" is used here before it's defined. Consider linking to the SSR definition or moving this explanation to after the SSR section is introduced. The link on line 118 properly points to the hydration section, but readers may not yet understand what SSR means at this point in the document.
| margin-bottom: 0.75em; | ||
| font-weight: 600; | ||
| color: var(--ft-text); | ||
| } |
There was a problem hiding this comment.
Missing link styles for the internal anchor links (lines 118, 143, 151-154, 162-164, 184, 199, 229-230). These links should have styling consistent with other links in the codebase (using var(--ft-accent) color and hover effects). Without explicit styles, these links will use browser defaults which may not match the site's design system.
| } | |
| } | |
| /* Internal anchor link styles */ | |
| a[href^="#"] { | |
| color: var(--ft-accent); | |
| text-decoration: none; | |
| border-bottom: 1px solid transparent; | |
| transition: | |
| color 0.2s ease, | |
| border-color 0.2s ease; | |
| } | |
| a[href^="#"]:hover, | |
| a[href^="#"]:focus-visible { | |
| color: var(--ft-accent); | |
| border-bottom-color: var(--ft-accent); | |
| } |
First draft for the glossary page to resolve #134
TODOs:
Checklist
For backlink:
https://github.com/user-attachments/assets/9a7ecb4e-4ec4-4e68-b673-dce73a5e0532
starter-*package, ensure that the deps, and file content matches the output of the metaframeworks recommended starter/default project