650-feat: Add docs #683
650-feat: Add docs #683
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive documentation system for the RS School website, integrating multilingual documentation with search functionality. It adds support for both English and Russian documentation, creates a flexible layout for rendering markdown content, and implements a client-side search feature using Pagefind. The changes span multiple components, utilities, and configuration files to provide a seamless documentation experience. Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (4)
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (15)
src/app/docs/utils/fetchMenu.ts (1)
9-10: Awaiting JSON
Returning parsed JSON is correct. Consider basic error handling or fallback for partial or invalid menu data.src/app/docs/utils/fetchMarkdownContent.ts (2)
3-5: Use a fallback or log for debugging.
When the file URL is formed, consider logging or providing additional context to debug issues if a markdown file path is missing or mislabeled.
15-16: Enhance error message with context.
Providing slightly more context (e.g., file URL) in the error message can greatly simplify debugging if the fetch fails.src/app/docs/[lang]/page.tsx (1)
23-24: Add error handling for missing menu entries.
If fetchMenu fails or returns an empty structure, consider a fallback UI or an error state to guide the user.src/app/docs/components/docs-menu/docs-menu.tsx (2)
38-66: Minor naming improvement
Consider renaming “doc” to “item” for clarity and consistency when rendering menu items.- return items.map((doc, index) => { + return items.map((item, index) => { if (item?.items) { ...
70-78: Add ARIA attribute for accessibility
Consider using aria-controls on the toggle button to indicate which element is being controlled.<button className={cx('menu-toggle')} onClick={handleMenuToggle} + aria-controls="docs-menu-nav" >src/app/docs/[lang]/[...slug]/page.tsx (2)
13-48: Consider a fallback title
If no matching item is found, the returned metadata title might be undefined. A default fallback would help.- return { title: `${title} ${TITLE_POSTFIX}` }; + return { title: title ? `${title} ${TITLE_POSTFIX}` : `Documentation ${TITLE_POSTFIX}` };
50-90: Potential optimization
While this logic is solid, you might combine repeated patterns or handle potentially empty items gracefully with optional chaining.src/app/docs/components/docs-layout/docs-layout.tsx (1)
67-75: Consider adding alt text or fallback behavior for images.For improved accessibility, specify the alt text for images. At the moment, the transformation logic for external/non-external image paths is good, but an empty alt attribute might degrade the accessibility experience.
- return <img {...props} src={newSrc} />; + return <img {...props} src={newSrc} alt="Documentation Image" />;src/app/docs/components/search/types.ts (1)
19-56: Structured Pagefind index options
Provided index options are well-typed and cover essential configuration. Consider adding JSDoc for each property if more clarity is needed for other team members.src/app/docs/mocked_search.ts (1)
341-499: Mock data item 3
Generally consistent with the previous entries. If you plan to expand heavily, consider externalizing the text for maintainability.src/app/docs/components/docs-layout/docs-layout.module.scss (3)
4-6: Consider using explicit width valueUsing
initialfor width might lead to inconsistent layouts across browsers. Consider setting a specific width or using a more explicit value likeautoor100%.
15-15: Extract magic number to a variableThe value
104pxis used multiple times. Consider extracting it to a SCSS variable for better maintainability.+$header-height: 104px; + .menu { position: sticky; - top: 104px; + top: $header-height; height: calc(100vh - 104px); + height: calc(100vh - $header-height);Also applies to: 20-20
25-32: Simplify tablet media query overridesInstead of using
initialmultiple times, consider using a singleall: initialor removing the properties entirely.src/core/styles/_constants.scss (1)
43-46: LGTM! Consider documenting color usageThe new color variables are well-structured. Consider adding comments to document their intended usage in the documentation UI.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/shared/assets/svg/chevron-right.svgis excluded by!**/*.svgsrc/shared/assets/svg/link.svgis excluded by!**/*.svgsrc/shared/assets/svg/translate.svgis excluded by!**/*.svg
📒 Files selected for processing (29)
eslint.config.js(1 hunks)package.json(2 hunks)src/app/docs/[lang]/[...slug]/page.tsx(1 hunks)src/app/docs/[lang]/page.tsx(1 hunks)src/app/docs/components/docs-layout/docs-layout.module.scss(1 hunks)src/app/docs/components/docs-layout/docs-layout.tsx(1 hunks)src/app/docs/components/docs-menu/docs-menu.module.scss(1 hunks)src/app/docs/components/docs-menu/docs-menu.tsx(1 hunks)src/app/docs/components/lang-switcher/lang-switcher.module.scss(1 hunks)src/app/docs/components/lang-switcher/lang-switcher.tsx(1 hunks)src/app/docs/components/search/search.module.scss(1 hunks)src/app/docs/components/search/search.tsx(1 hunks)src/app/docs/components/search/types.ts(1 hunks)src/app/docs/constants.ts(1 hunks)src/app/docs/mocked_search.ts(1 hunks)src/app/docs/types.ts(1 hunks)src/app/docs/utils/fetchMarkdownContent.ts(1 hunks)src/app/docs/utils/fetchMenu.ts(1 hunks)src/core/base-layout/components/header/burger/burger.module.scss(0 hunks)src/core/base-layout/components/header/header.module.scss(1 hunks)src/core/base-layout/components/header/header.test.tsx(1 hunks)src/core/base-layout/components/header/header.tsx(2 hunks)src/core/base-layout/components/header/nav-item/nav-item.tsx(1 hunks)src/core/const/index.ts(1 hunks)src/core/styles/_constants.scss(1 hunks)src/core/styles/index.scss(2 hunks)src/core/types/route.types.ts(1 hunks)src/shared/helpers/isValidUrl.ts(1 hunks)src/widgets/mobile-view/ui/mobile-view.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/core/base-layout/components/header/burger/burger.module.scss
✅ Files skipped from review due to trivial changes (3)
- src/app/docs/components/lang-switcher/lang-switcher.module.scss
- src/app/docs/constants.ts
- src/app/docs/components/docs-menu/docs-menu.module.scss
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/docs/components/search/search.tsx
[error] 124-124: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 131-132: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (48)
src/app/docs/types.ts (2)
1-5: Good use of optional properties
These type definitions are straightforward and effectively accommodate nested menus.
7-9: Export matches usage
Exporting the Menu type ensures consistent usage throughout the docs components.
src/core/types/route.types.ts (1)
5-5: Confirm route exclusions
Excluding these additional paths looks correct. Verify other code references properly handle or skip 'docs/en' and 'docs/ru' as intended.
✅ Verification successful
Let me search for references to these route paths and analyze how they are handled.
Let's check the actual implementation of Route type and how these paths are handled in the core files.
Route exclusions are properly handled
The exclusion of 'docs/en' and 'docs/ru' paths in RouteValues type is correct and consistent with the codebase. These paths are defined in ROUTES constant and are used specifically for documentation purposes, as evidenced by their usage in the mocked search data and documentation-related files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for references to 'docs/en' and 'docs/ru' in the codebase
rg -l "docs/[er][nu]"
# Search for Route type usage and related route handling
ast-grep --pattern 'type Route = {
$$$
}'
# Look for route-related constants or configurations
rg -l "Route[^a-zA-Z]"
Length of output: 871
Script:
#!/bin/bash
# Check the Route type implementation and related constants
cat src/core/types/route.types.ts
cat src/core/const/index.ts
# Check how these routes are used in the mocked search
cat src/app/docs/mocked_search.ts
Length of output: 41713
src/shared/helpers/isValidUrl.ts (1)
1-12: Straightforward URL validation
This function cleanly checks for valid HTTP(S) protocols. If other URL schemes become relevant, consider extending the protocol check.
src/app/docs/utils/fetchMenu.ts (1)
5-7: Store the base URL in a constant
Repeating the same link segment can become error-prone. As previously noted:
src/app/docs/utils/fetchMarkdownContent.ts (2)
7-11: Consider validation for slug array length.
If there’s a possibility that the slug contains more than two elements, account for that—or throw a meaningful error to prevent unexpected fetches.
19-20: Confirm content type
Ensure that the endpoint always returns valid text-based content. Consider verifying the Content-Type header and gracefully handling non-Markdown responses.
src/app/docs/[lang]/page.tsx (2)
9-11: Metadata usage looks good.
The dynamic title appending is straightforward; no issues found here.
13-18: Prerendering multiple language pages is a solid approach.
No issues are apparent, and this method simplifies the multilingual setup.
src/app/docs/components/lang-switcher/lang-switcher.tsx (2)
20-30: No critical remarks—implementation is concise.
Your approach for language switching is straightforward and user friendly.
15-16: Check for potential false positives.
For deeply nested paths beyond "docs/en" or "docs/ru", verify that startsWith remains reliable for determining active states.
✅ Verification successful
Let me verify the concern by checking how the routes are defined and used in the codebase.
This will help us understand:
- How ROUTES are defined and what values DOCS_RU and DOCS_EN have
- Where these route constants are used
- Any potential path matching patterns that might cause issues
No issues with startsWith route matching
The implementation is correct and reliable because:
- ROUTES.DOCS_EN and DOCS_RU are defined as 'docs/en' and 'docs/ru' respectively
- These route constants are used consistently across the codebase
- The
startsWithcheck will correctly identify the language section regardless of nested paths, as it only checks the beginning of the path
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for ROUTES constant definition
rg "ROUTES\s*=\s*" -A 5
# Find all usages of DOCS_RU and DOCS_EN
rg "DOCS_[RE][NU]" -A 2
# Look for route handling patterns in the codebase
ast-grep --pattern 'startsWith($$$)'
Length of output: 2873
src/core/const/index.ts (1)
37-38: Routes are straightforward.
The new entries for DOCS_EN and DOCS_RU look correct and maintainable.
src/app/docs/components/docs-menu/docs-menu.tsx (3)
1-2: Everything looks good here!
No issues detected with the client-side rendering directive.
30-32: Use startsWith for more flexible matching
This mirrors a previously mentioned suggestion to handle possible query parameters or trailing slashes.
34-36: Good approach for external vs. internal links
The resolveHref function is clean and straightforward.
src/core/base-layout/components/header/nav-item/nav-item.tsx (1)
33-33: Straightforward route matching
Ensures an exact match for '/' and uses startsWith for subroutes.
src/core/base-layout/components/header/header.test.tsx (1)
39-39: Expanded number of header links
Increasing the expected length from 4 to 5 likely accounts for the newly added “Docs” link.
src/app/docs/[lang]/[...slug]/page.tsx (2)
10-11: Well-defined route parameters
Using a dedicated type for language and slug arrays keeps the code organized.
92-108: Exception handling is robust
The try/catch with notFound() is clear and meets the requirement for handling missing content.
src/widgets/mobile-view/ui/mobile-view.tsx (1)
109-114: Smooth addition of the "Docs" link.
The newly added link to "Docs" logically follows the existing navigation pattern and uses consistent styling. This appears ready to merge.
src/app/docs/components/docs-layout/docs-layout.tsx (1)
76-99: Verify that the link transformation covers all edge cases.
When removing ".md" extensions or adjusting the URL to avoid image linking, confirm that no other content types (e.g., PDFs, external resources, or anchors) break. Ensure robust error handling or fallback logic if transformations fail.
src/core/base-layout/components/header/header.tsx (2)
43-51: Good documentation on scroll behavior.
Your comment provides clarity on how the header color changes beyond 500px of scroll. This helps other contributors understand its purpose.
135-135: Docs link addition appears well-placed.
This addition aligns with the existing header items and helps users discover documentation easily. No issues detected.
eslint.config.js (1)
106-111: Enforce the JSON extension requirement thoroughly
Enforcing JSON imports to always include the file extension helps ensure clarity and consistency across the codebase. This is widely considered a best practice and reduces ambiguities involving possibly missing or ambiguous imports.
src/app/docs/components/search/types.ts (8)
88-98: Search options
Options such as preload and verbose are well understood. Ensure that the “filters” and “sort” objects remain flexible in usage but remain typed as soon as possible to avoid runtime errors.
100-119: Search results structure
Providing unfilteredResultCount and totalFilters looks beneficial. Ensure post-processing on the results array, if any, respects these metadata fields to maintain consistency.
139-176: Detailed fragments
The included excerpt, sub_results, and filters provide excellent context coverage. Nothing problematic found.
179-210: Subresult organization
This structure logically encapsulates heading-specific data. The approach is clear and fosters easy extension for future feature requests.
232-253: Anchor element metadata
Capturing anchor shape for subresult generation is helpful. This thorough approach might allow advanced UI features such as anchor-based navigation or highlighting.
5-16: Global “Window” extension clarity
Declaring the pagefind property on the Window interface is straightforward and ensures type safety. Verify that downstream usages of window.pagefind handle potential undefined states properly.
✅ Verification successful
Let me search for usages of window.pagefind to verify proper handling of undefined states.
Window.pagefind usage is properly guarded
The code handles undefined states correctly by checking if (window.pagefind) before accessing it, and provides a mock implementation as fallback. The type declaration accurately reflects the runtime behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for window.pagefind usages
rg "window\.pagefind" -A 3
Length of output: 1210
57-86: Fine-grained ranking weights
The ranking customization is comprehensive. Verify default values or fallback behavior in your actual search logic to avoid unexpected ranking outputs.
✅ Verification successful
Let me search for the search implementation to verify the default values and fallback behavior.
Let me check the search implementation to see how these weights are actually used.
Let me check if there's any configuration or initialization of these ranking weights.
Ranking weights are properly typed and optional
The ranking weights are correctly defined as optional properties in the PagefindRankingWeights type, and the search implementation uses the default Pagefind configuration. The weights will fall back to Pagefind's built-in defaults when not specified, ensuring consistent ranking behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for files related to search implementation
fd "search" --type f
# Look for pagefind configuration or initialization
rg -l "PagefindRankingWeights"
# Look for actual usage of ranking weights
rg "termSimilarity|pageLength|termSaturation|termFrequency" --type ts
Length of output: 564
Script:
#!/bin/bash
# Check the search implementation
cat src/app/docs/components/search/search.tsx
# Check the mocked search implementation
cat src/app/docs/mocked_search.ts
Length of output: 44079
Script:
#!/bin/bash
# Look for pagefind initialization or configuration
rg -A 5 "pagefind\.options|pagefind\.search" --type ts
Length of output: 2941
213-230: Word location indexing
Balanced scoring is a neat concept. Confirm that location indices align reliably with how the content is tokenized or whitespace-split in practice.
✅ Verification successful
Let me search for relevant code to verify the word location indexing implementation.
This will help us understand:
- How content is split/tokenized
- How location indices are used
- If there are tests verifying this behavior
Let me try a different search strategy to find the implementation details.
Let me search for the content processing implementation to verify how the location indices are used.
Based on the evidence from the mocked search results and type definitions, I can now provide a final response:
Location indices align with content reliably
The mocked search results demonstrate that location indices correctly map to words in the content, as shown by the excerpts containing marked words at the specified locations. For example, location indices [342, 344] correctly map to "Certificate. A certificate" in the excerpts. The implementation uses these indices to highlight matched terms with <mark> tags while preserving the original text structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for content tokenization or splitting implementation
rg -A 5 "content.*split|tokeniz.*content" --type ts
# Search for location index usage
rg -A 5 "location.*index|word.*location" --type ts
# Look for test files related to search functionality
fd "search.*test" -e ts -e js
Length of output: 138
Script:
#!/bin/bash
# Search for Pagefind-related implementations
rg -A 10 "PagefindWord" --type ts
# Look for search result processing
rg -A 10 "balanced_score|search.*result" --type ts
# Find files containing search-related types or implementations
fd "search|pagefind" -e ts
Length of output: 36994
Script:
#!/bin/bash
# Search for content processing or text splitting in search implementation
rg -A 5 "content.*split|process.*content|tokenize" src/app/docs/components/search/
# Look for excerpt generation code
rg -A 5 "excerpt.*generate|mark.*word" src/app/docs/components/search/
Length of output: 1244
src/app/docs/mocked_search.ts (3)
182-340: Mock data item 2
The second mocked item is similarly well-structured, including detailed anchors. Looks fine.
1-181: Mock data item 1
This entry gives users a realistic certificate example. Keep verifying that no personal data is accidentally exposed.
✅ Verification successful
No sensitive data exposed in mock search results
The mock data only contains public course information and general requirements for obtaining certificates. The only contact information is a Discord handle (@davojta), which is a public point of contact. No personal data patterns were found in the search results or related mock files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for any potential personal data patterns in mock data files
rg -i "(\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b|phone|address|ssn|social security|passport|credit card)" src/app/docs/
# Look for other mock data files that might contain sensitive information
fd -e ts -e js "mock" src/app/docs/
Length of output: 3084
501-609: Mock data item 4
Ensure that the excerpt references remain accurate if the text is updated. This example also carefully includes nested headings, which is beneficial for thorough testing.
✅ Verification successful
Mock data structure is correctly implemented and used
The mock data is only used in development environment as a fallback for the Pagefind search functionality, and the structure matches the expected interface. The excerpts and nested headings are correctly referenced in the Result component's rendering logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for similar mock data structures in the codebase
rg -U "id: 'en_[a-f0-9]+'" -A 10 -B 10
# Look for any other files that might be using or referencing this mock data
rg -l "mocked_search"
Length of output: 15140
Script:
#!/bin/bash
# Check how the mock data is used in the search component
cat src/app/docs/components/search/search.tsx
Length of output: 3787
src/app/docs/components/search/search.module.scss (7)
1-5: .search container
Flex layout with column direction is a clean approach to keep contents stacked. Nothing to change.
7-19: .search-input styling
Reasonable overall. The 5px border-radius is subtle and modern; no concerns here.
21-23: .no-results notice
Margin spacing is appropriate. The class name is semantically meaningful.
25-29: .subresults indentation
The vertical border is a helpful visual separator.
31-33: .subresult spacing
Light margin ensures clarity between each subresult block.
35-37: .subresult h4
Zero margin is neat, aligning well with the rest of the design.
39-41: .subresult p
A 14px font size is generally readable without crowding.
src/app/docs/components/docs-layout/docs-layout.module.scss (1)
35-47: LGTM!
Clean implementation with proper responsive adjustments.
src/core/base-layout/components/header/header.module.scss (1)
2-2: LGTM!
Position sticky is appropriate for the documentation header.
src/core/styles/index.scss (3)
2-2: LGTM: GitHub markdown styles import
Good choice using github-markdown-css for consistent documentation styling.
152-155: LGTM: List style positioning
Proper adjustment for better list readability.
157-176: Verify the link icon asset path
The icon path seems to be relative. Ensure the asset exists at ../../shared/assets/svg/link.svg.
✅ Verification successful
Link icon asset path verified successfully
The link.svg file exists at the expected location src/shared/assets/svg/link.svg, confirming the relative path in the stylesheet is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the link icon exists
fd -t f "link.svg" src/shared/assets/svg
Length of output: 107
package.json (1)
30-44: LGTM: Documentation dependencies
Good selection of markdown processing packages for enhanced documentation features:
- github-markdown-css for styling
- react-markdown for rendering
- remark/rehype plugins for extended functionality
|
Lighthouse Report:
|
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/docs/components/search/search.tsx (2)
31-34: Consider making the RefObject type more specificThe
RefObject<null>type doesn't provide much value. Consider simplifying to justRefObject<HTMLDivElement>.type SearchProps = { lang: Language; - resultsRef: RefObject<null> | RefObject<HTMLDivElement>; + resultsRef: RefObject<HTMLDivElement>; };
113-120: Enhance URL cleaning robustnessThe URL cleaning could handle more edge cases and be more defensive.
const removeHtmlExtension = (url: string): string => { + if (!url) return ''; const urlObj = new URL(url, window.location.origin); const hash = urlObj.hash; const pathname = urlObj.pathname; - const cleanedPathname = pathname.endsWith('.html') ? pathname.slice(0, -5) : pathname; + const cleanedPathname = pathname.toLowerCase().endsWith('.html') + ? pathname.slice(0, -5) + : pathname; return `${cleanedPathname}${hash}`; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/docs/components/search/search.tsx(1 hunks)src/core/styles/index.scss(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/styles/index.scss
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/docs/components/search/search.tsx
[error] 133-133: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 140-141: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
|
Lighthouse Report:
|
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
Adds the documentation page
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Chores
package.json.