chore: upgrade alauda doom #89
Conversation
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
WalkthroughAdds Card and LinkCard components, a DocLayout plus sidebar hook, download helpers with MDX anchor interception, large CSS additions (tokens and doc-layout), TS config and virtual-module export changes, multiple import-source swaps to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (click)
participant MDX as Layout/MDX anchor handler
participant Util as download.ts
participant Net as Network (fetch / browser)
note right of MDX: Anchor click intercepted\nby MDX anchor handler
User->>MDX: click anchor (href)
MDX->>Util: getPathname(href)
MDX->>Util: shouldDownload(pathname)?
alt shouldDownload == false
MDX->>User: allow normal navigation
else shouldDownload == true
MDX->>Util: downloadFile(href, filename)
alt same-origin
Util->>Net: fetch(normalized URL)
Net-->>Util: Response (ok/blob)
Util->>User: create blob URL & programmatic click (download)
Util->>Net: revoke blob URL (cleanup)
else cross-origin
Util->>User: create temp anchor href & programmatic click (download)
Util->>User: cleanup temp anchor
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
theme/layout/index.tsx (1)
9-9: Consider removing explicit React import.With React 17+ and the JSX transform configured in tsconfig.json (
"jsx": "react-jsx"), the explicitimport React from "react"is unnecessary. However, if you prefer to keep it for consistency or compatibility reasons, this is a minor stylistic choice.theme/components/SidebarMenu/useSidebarMenu.tsx (1)
40-40: Consider extracting the mobile breakpoint as a constant.The hardcoded
768breakpoint appears here and might be used elsewhere in the theme. Consider extracting it to a shared constant or configuration file for maintainability.// Example: theme/constants.ts or similar export const MOBILE_BREAKPOINT = 768; // Then import and use: const isMobile = window.innerWidth <= MOBILE_BREAKPOINT;theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing explicit React import.With the JSX transform configured in tsconfig.json (
"jsx": "react-jsx"), the explicitimport React from "react"is unnecessary. However, this is a minor stylistic preference.theme/components/HomeContent/index.tsx (1)
21-21: Optional: Explicit React import may be unnecessary.Since React 17+, the explicit
import React from "react"is not required for JSX transformation. The file already imports React hooks on Line 3, which is sufficient.If you prefer to keep imports minimal, you can remove this line:
-import React from "react";theme/components/Card/index.tsx (1)
3-9: Consider clarifying the content vs. children prop usage.The Card component accepts both
contentandchildrenprops, and both will be rendered if provided (lines 28-29). While this offers flexibility, it may lead to confusion about which prop to use.Consider either:
- Documenting the intended usage pattern (e.g., "use
contentfor simple content,childrenfor complex layouts")- Or making them mutually exclusive if that's the intended behavior
If mutual exclusivity is preferred, you could apply this pattern:
export const Card: React.FC<CardProps> = ({ title, content, style, className = "", children, }) => { return ( <div className={`rp-border rp-block rp-border-solid rp-border-divider-light rp-rounded-lg rp-bg-bg rp-p-4 ${className}`} style={style} > {title && ( <div className="rp-text-lg rp-font-semibold rp-mb-3 rp-text-text-1"> {title} </div> )} - {content && <div className="rp-text-text-2">{content}</div>} - {children && <div className="rp-text-text-2">{children}</div>} + <div className="rp-text-text-2">{content || children}</div> </div> ); };Alternatively, keep the current flexible approach but add a JSDoc comment to clarify the intended usage.
Also applies to: 28-29
theme/layout/DocLayout/index.tsx (1)
47-48: Consider making sidebar/footer visibility configurable or removing dead code.Lines 47-48 hardcode
showSidebar = falseandshowDocFooter = false, which makes several code paths unreachable:
- The sidebar rendering logic (lines 70-88) will always render the placeholder
- The
DocFooter(line 116) will never be rendered- The
beforeSidebar,afterSidebar,beforeDocFooter, andafterDocFooterprops inDocLayoutPropsbecome effectively unusedWhile the Chinese comments indicate this is intentional ("强制隐藏" = force hide), consider either:
Making these configurable by adding them to
DocLayoutProps:export interface DocLayoutProps { showSidebar?: boolean; showDocFooter?: boolean; // ... other props }Or removing the dead code entirely to improve maintainability:
- Remove unused conditional branches
- Remove unused props from the interface
- Simplify the component logic
The current approach leaves dead code paths that may confuse future maintainers.
Also applies to: 70-88, 116-116
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
package.json(1 hunks)styles/index.css(1 hunks)theme/components/Card/index.tsx(1 hunks)theme/components/Empty/index.tsx(1 hunks)theme/components/HomeContent/index.tsx(2 hunks)theme/components/LinkCard/index.tsx(1 hunks)theme/components/PostList/index.tsx(2 hunks)theme/components/SidebarMenu/index.ts(1 hunks)theme/components/SidebarMenu/useSidebarMenu.tsx(1 hunks)theme/index.tsx(1 hunks)theme/layout/DocLayout/index.tsx(1 hunks)theme/layout/HomeLayout/index.tsx(1 hunks)theme/layout/index.tsx(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/components/LinkCard/index.tsx (1)
theme/index.tsx (1)
LinkCard(11-11)
theme/layout/DocLayout/index.tsx (3)
theme/index.tsx (1)
DocLayout(5-5)theme/components/SidebarMenu/index.ts (1)
useSidebarMenu(1-1)theme/components/SidebarMenu/useSidebarMenu.tsx (1)
useSidebarMenu(4-124)
theme/components/PostList/index.tsx (1)
plugins/plugin-post-resolver/types.ts (1)
PostInfo(5-16)
🪛 Biome (2.1.2)
styles/index.css
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (15)
theme/components/SidebarMenu/index.ts (1)
1-2: LGTM!Clean re-export pattern for the new sidebar menu hook.
theme/layout/index.tsx (1)
2-3: Verify theme-original exports are available.The imports have been switched from
@rspress/core/themeto@rspress/core/theme-original. Ensure thatBadge,LastUpdated, andLayoutare properly exported from the theme-original module in the upgraded version.theme/components/PostList/index.tsx (1)
2-3: Import updates align with PR objectives.The migration to
theme-originalfor Badge and the addition of LinkCard import from the relative path are consistent with the broader refactoring in this PR.Also applies to: 9-10
tsconfig.json (1)
13-14: LGTM! i18n alias supports improved type safety.The new path alias enables type-safe i18n imports as seen in PostList. The alias correctly points to
./docs/i18n.json, andresolveJsonModuleis already enabled in the compiler options.theme/components/SidebarMenu/useSidebarMenu.tsx (1)
50-63: Well-structured hook with clear separation of concerns.The toggle functions correctly ensure mutually exclusive visibility between sidebar and outline, and the generated menu JSX includes proper ARIA attributes for accessibility. The returned API is comprehensive and well-organized.
Also applies to: 66-113, 115-124
package.json (1)
5-5: Verify that version 1.17.1 of @alauda/doom exists.The npm registry currently shows version 1.12.7 as the latest available version of @alauda/doom. Before proceeding with this upgrade, confirm that version 1.17.1 has been released and is accessible in your configured npm registry. If it exists, review the changelog for breaking changes and verify support for the theme-original import pattern in that specific version.
theme/components/HomeContent/index.tsx (2)
2-2: LGTM: Consistent migration to theme-original.The import source change from
@rspress/core/themeto@rspress/core/theme-originalaligns with the PR's broader refactoring to use theme-original APIs.
16-16: LGTM: Using the new local Card component.The local Card import from
../Cardcorrectly integrates with the newly introduced Card component, ensuring consistency across the updated theme architecture.theme/components/LinkCard/index.tsx (1)
1-35: LGTM: Well-structured LinkCard component.The LinkCard component is cleanly implemented with:
- Proper TypeScript interface definition
- Appropriate use of semantic HTML (
<a>tag)- Flexible props for customization (title, description, style, className)
- Integration with the new RPpress v2 utility classes
The component follows React best practices and integrates well with the theme architecture.
theme/index.tsx (3)
3-3: LGTM: DocLayout properly integrated into theme exports.The DocLayout import and export expand the public theme API appropriately, making the new layout component available alongside existing layouts.
Also applies to: 5-5
7-7: LGTM: Consistent theme-original migration.The change from
@rspress/core/themeto@rspress/core/theme-originalaligns with the PR's broader migration strategy and maintains consistency across all theme files.
9-11: LGTM: Local components properly exposed.The Card and LinkCard components are correctly exported from their respective modules, making them available as part of the public theme API.
styles/index.css (2)
22-23: False positive: Lines 22-23 are not duplicate properties.The static analysis tool incorrectly flagged these lines as duplicates. They serve different purposes:
- Line 22 defines a CSS custom property (
--tw-font-weight)- Line 23 applies the actual
font-weightstyle propertyBoth are necessary and not duplicates.
57-308: LGTM: Comprehensive and well-organized CSS utilities.The RPpress v2 utility classes are well-structured with:
- Clear organization by category (layout, spacing, borders, typography, etc.)
- Consistent naming convention with
rp-prefix- Proper responsive and dark mode support
- Integration with CSS custom properties for theming
The utilities effectively support the new Card, LinkCard, and DocLayout components introduced in this PR.
theme/layout/DocLayout/index.tsx (1)
1-13: LGTM: Well-structured DocLayout implementation.The DocLayout component demonstrates good architectural patterns:
- Clear props interface with flexible before/after hooks for customization
- Proper integration with rspress theme-original components
- Effective use of React hooks (useFrontmatter, useWatchToc, useSidebarMenu)
- Responsive design with mobile menu support via useSidebarMenu
- Conditional rendering based on page type and frontmatter configuration
The component provides a solid foundation for document layout with appropriate customization points.
Also applies to: 14-46, 49-69, 89-115, 117-147
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
theme/components/PostList/index.tsx (1)
49-53: Missing i18n key already flagged in previous review.The missing
lastUpdatedTexttranslation key has already been identified in a previous review. Please ensure this key is added todocs/i18n.jsonwith translations for all supported locales.
🧹 Nitpick comments (3)
theme/components/SidebarMenu/useSidebarMenu.tsx (2)
40-40: Address or clarify the TODO comment.The TODO comment lacks context about what needs discussion. Consider either resolving the issue or adding more detail about what requires discussion.
Do you want me to help identify what might need discussion here, or would you like to open an issue to track this?
19-24: Consider memoizing the callback and JSX for better performance.The
toggleOutlinefunction andsidebarMenuJSX are recreated on every render, which could cause unnecessary re-renders in consuming components.Apply these optimizations:
-import { useRef, useState } from "react"; +import { useRef, useState, useCallback, useMemo } from "react"; import { useI18n } from "@rspress/core/runtime"; import { useDynamicToc, ReadPercent, SvgWrapper, IconArrowRight, } from "@rspress/core/theme-original"; export function useSidebarMenu() { const [isSidebarOpen, setIsSidebarOpen] = useState(false); const [isOutlineOpen, setIsOutlineOpen] = useState(false); const sidebarLayoutRef = useRef<HTMLElement>(null); const asideLayoutRef = useRef<HTMLElement>(null); const headers = useDynamicToc(); const t = useI18n(); - const toggleOutline = () => { + const toggleOutline = useCallback(() => { setIsOutlineOpen((prev) => !prev); if (isSidebarOpen) { setIsSidebarOpen(false); } - }; + }, [isSidebarOpen]); - const sidebarMenu = ( + const sidebarMenu = useMemo(() => ( <div className="rp-doc-layout__menu"> <button type="button" disabled={headers.length === 0} onClick={(e) => { e.preventDefault(); e.stopPropagation(); toggleOutline(); }} className="rp-sidebar-menu__right" > <span>{t("outlineTitle")}</span> <ReadPercent size={14} strokeWidth={2} /> {/* TODO: discussion */} {headers.length !== 0 && ( <SvgWrapper icon={IconArrowRight} style={{ transform: isOutlineOpen ? "rotate(90deg)" : "rotate(0deg)", transition: "transform 0.2s ease-out", }} /> )} </button> </div> - ); + ), [headers.length, isOutlineOpen, toggleOutline, t]); return { isSidebarOpen, isOutlineOpen, sidebarMenu, asideLayoutRef, sidebarLayoutRef, toggleOutline, }; }Also applies to: 26-52
theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing the unused React import.With
"jsx": "react-jsx"configured intsconfig.json, the explicitReactimport is unnecessary. The JSX transform handles this automatically.Apply this diff to remove the unused import:
-import React from "react";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
styles/index.css(1 hunks)theme/components/BreadCrumb/index.tsx(2 hunks)theme/components/Card/index.tsx(1 hunks)theme/components/Checkbox/index.tsx(0 hunks)theme/components/DocID/index.tsx(1 hunks)theme/components/EditOnGithub/index.tsx(1 hunks)theme/components/HomeBanner/index.tsx(2 hunks)theme/components/HomeContent/index.tsx(3 hunks)theme/components/LinkCard/index.tsx(1 hunks)theme/components/Pagination/index.tsx(1 hunks)theme/components/PostList/index.tsx(2 hunks)theme/components/SidebarMenu/useSidebarMenu.tsx(1 hunks)theme/layout/HomeLayout/index.tsx(2 hunks)theme/layout/index.tsx(1 hunks)tsconfig.json(1 hunks)virtual-modules.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- theme/components/Checkbox/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- theme/components/LinkCard/index.tsx
- theme/layout/index.tsx
- theme/components/HomeContent/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/components/PostList/index.tsx (2)
virtual-modules.d.ts (1)
PostInfo(3-14)plugins/plugin-post-resolver/types.ts (1)
PostInfo(5-16)
🪛 Biome (2.1.2)
styles/index.css
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (18)
theme/components/Pagination/index.tsx (1)
18-18: LGTM: Type safety improvement for i18n.The explicit generic type parameter aligns with the new
"i18n"path alias intsconfig.jsonand provides better type inference for translation keys.theme/components/EditOnGithub/index.tsx (1)
46-46: LGTM: Consistent i18n typing pattern.The typing update is consistent with other components in this PR and improves type safety.
theme/components/HomeBanner/index.tsx (1)
12-12: LGTM: Consistent i18n typing.The explicit type parameter follows the same pattern as other components in this PR.
tsconfig.json (1)
12-22: LGTM: TypeScript configuration supports the i18n typing pattern.The addition of
baseUrland the"i18n"path alias correctly supports the typeduseI18n<typeof import("i18n")>()pattern used throughout the codebase. The expandedincludepatterns appropriately cover the theme and plugin directories.theme/layout/HomeLayout/index.tsx (1)
24-28: LGTM: Footer styling aligns with CSS theming updates.The simplified class names (
home-footer,home-footer__content) align with the token-based theming approach introduced instyles/index.css.virtual-modules.d.ts (1)
1-26: LGTM: Well-structured type declarations for virtual modules.The type declarations properly expose
PostInfoand related exports for virtual modules. ThePostInfointerface is consistent withplugins/plugin-post-resolver/types.ts.theme/components/PostList/index.tsx (2)
1-9: LGTM: Import reorganization aligns with theme refactoring.The import changes correctly source
Badgefromtheme-originaland add the necessaryLinkCarddependency, consistent with the broader theme reorganization in this PR.
17-17: LGTM: Consistent i18n typing pattern.theme/components/Card/index.tsx (1)
3-9: LGTM: Well-defined CardProps interface.The interface provides a flexible API with optional properties for various use cases.
styles/index.css (6)
57-74: LGTM: Card component styling.The card component styling is well-structured with consistent use of CSS variables for theming. The separation of card, card__title, and card__content provides good modularity.
76-103: LGTM: Link card component styling.The link-card component is well-implemented with smooth hover transitions and consistent theming. The use of
border-bottominstead of a full border is appropriate for list-style layouts.
105-117: LGTM: Post-meta styling aligns with component refactor.The post-meta styling correctly replaces the utility classes previously used in the DocID component, providing centralized and maintainable styling with responsive behavior.
119-121: LGTM: Breadcrumb styling aligns with component refactor.The breadcrumb styling appropriately replaces the
rp-mb-10utility class previously used in the BreadCrumb component.
123-146: LGTM: Home footer styling.The home-footer component styling is well-implemented with proper responsive behavior and consistent theming through CSS variables.
148-194: LGTM: Responsive document layout menu.The rp-doc-layout__menu styling is properly implemented with:
- Appropriate fixed positioning and z-index
- Responsive display behavior (visible only below 1280px)
- Well-defined interactive states for the menu button
theme/components/BreadCrumb/index.tsx (2)
7-7: LGTM: Improved type safety with typed i18n hook.Adding the type parameter to
useI18nprovides better type checking and IDE support, aligning with the PR-wide typing improvements.
17-17: LGTM: Semantic class name aligns with CSS refactor.The className change from
rp-mb-10tobreadcrumbaligns with the new centralized styling instyles/index.cssand follows semantic naming conventions.theme/components/DocID/index.tsx (1)
11-11: LGTM: Consolidates utility classes into semantic class.The className change from multiple utility classes to the single
post-metaclass improves maintainability while preserving the visual styling. The new class instyles/index.cssprovides equivalent responsive behavior.
yangxiaolang
force-pushed
the
chore/upgrade-alauda-doom
branch
from
efca7a3 to
9b9f20d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
theme/components/HomeContent/index.tsx (1)
126-130: MissingonSearchParamsChangein dependency array.The
onPageChangecallback usesonSearchParamsChangebut has an empty dependency array, which may cause stale closure issues.🔎 Proposed fix
const onPageChange = useCallback( (number: number) => onSearchParamsChange(new URLSearchParams({ page: `${number}` })), - [] + [onSearchParamsChange] );
♻️ Duplicate comments (4)
theme/components/HomeBanner/index.tsx (1)
1-1: Unresolved: Import path inconsistency remains.This import path inconsistency was flagged in a previous review and has not been addressed. All other components in the project import from
@rspress/core/runtime, but this file imports from@rspress/runtime. Update to maintain consistency across the codebase.🔎 Proposed fix
-import { useI18n } from "@rspress/runtime"; +import { useI18n } from "@rspress/core/runtime";theme/components/Card/index.tsx (1)
3-9: Unresolved: Ambiguouscontentvschildrenprop usage.This design issue was flagged in a previous review and remains unaddressed. The component accepts both
contentandchildrenprops and renders them into separate<div className="card__content">containers (lines 21-22), which creates confusing behavior and potential styling conflicts when both are provided.Make
contentandchildrenmutually exclusive to clarify the component's API.🔎 Recommended fix (Option 1: Mutually exclusive)
export const Card: React.FC<CardProps> = ({ title, content, style, className = "", children, }) => { return ( <div className={`card ${className}`} style={style}> {title && <div className="card__title">{title}</div>} - {content && <div className="card__content">{content}</div>} - {children && <div className="card__content">{children}</div>} + {(content || children) && ( + <div className="card__content">{content || children}</div> + )} </div> ); };Also applies to: 21-22
styles/index.css (1)
16-30: Remove duplicatefont-weightdeclaration.Line 17 declares
font-weight: 500and line 23 declares it again with the CSS variable. The second declaration overrides the first, making line 17 redundant.theme/components/PostList/index.tsx (1)
49-53: EnsurelastUpdatedTextkey exists in i18n configuration.The code uses
t("lastUpdatedText")but a previous review flagged that this key may be missing from the i18n configuration.
🧹 Nitpick comments (2)
theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing the redundant React import.With React 17+ and the new JSX transform, explicitly importing React is no longer necessary for JSX usage. This import can be safely removed.
🔎 Proposed cleanup
import { usePageData } from "@rspress/runtime"; import { ReactNode } from "react"; import { HomeBanner } from "../../components/HomeBanner"; import { HomeContent } from "../../components/HomeContent"; -import React from "react";theme/components/HomeContent/index.tsx (1)
21-21: Consider moving React import to the top of the file.The React import is placed after other imports. Conventionally, React imports should be at the top, before other dependencies.
🔎 Suggested reorder
+import React from "react"; import { useI18n, usePageData } from "@rspress/core/runtime"; import { useFullTextSearch } from "@rspress/core/theme-original"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { PostInfo, postInfos } from "virtual-post-data"; ... -import React from "react";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
package.jsonstyles/index.csstheme/components/BreadCrumb/index.tsxtheme/components/Card/index.tsxtheme/components/Checkbox/index.tsxtheme/components/DocID/index.tsxtheme/components/EditOnGithub/index.tsxtheme/components/Empty/index.tsxtheme/components/HomeBanner/index.tsxtheme/components/HomeContent/index.tsxtheme/components/LinkCard/index.tsxtheme/components/Pagination/index.tsxtheme/components/PostList/index.tsxtheme/components/SidebarMenu/index.tstheme/components/SidebarMenu/useSidebarMenu.tsxtheme/index.tsxtheme/layout/DocLayout/index.tsxtheme/layout/HomeLayout/index.tsxtheme/layout/index.tsxtheme/utils/download.tstsconfig.jsonvirtual-modules.d.ts
💤 Files with no reviewable changes (1)
- theme/components/Checkbox/index.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- theme/components/LinkCard/index.tsx
- theme/components/BreadCrumb/index.tsx
- tsconfig.json
- theme/layout/DocLayout/index.tsx
- theme/components/SidebarMenu/useSidebarMenu.tsx
- theme/components/Pagination/index.tsx
- theme/components/Empty/index.tsx
- theme/components/SidebarMenu/index.ts
- theme/components/EditOnGithub/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/layout/index.tsx (1)
theme/utils/download.ts (3)
getPathname(16-22)shouldDownload(11-14)downloadFile(24-57)
🪛 Biome (2.1.2)
styles/index.css
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (10)
theme/components/DocID/index.tsx (1)
11-11: LGTM! Semantic className improves maintainability.The refactoring from utility classes to the semantic
post-metaclassName aligns with the broader styling standardization in this PR and improves code maintainability.theme/components/HomeBanner/index.tsx (1)
12-12: LGTM! Type parameter improves type safety.Adding the type parameter to
useI18nstrengthens type checking and improves developer experience with better autocomplete.theme/layout/HomeLayout/index.tsx (1)
24-28: LGTM! Footer refactoring improves maintainability.The simplified footer structure with semantic class names (
home-footer,home-footer__content) aligns well with the styling standardization throughout this PR.virtual-modules.d.ts (1)
1-25: LGTM!The virtual module type declarations are well-structured with proper exports for
PostInfointerface and the related constants. The typing provides good type safety for consuming components.styles/index.css (2)
57-103: LGTM!The
.cardand.link-cardcomponent styles are well-implemented with consistent use of CSS variables for theming, proper transitions, and hover states that align with the new Card and LinkCard components.
148-194: LGTM!The responsive menu styles are well-structured with appropriate breakpoint handling, consistent theming via CSS variables, and proper interactive states (hover, active).
theme/components/HomeContent/index.tsx (1)
44-72: Dependency array may be incomplete forsearchFull.The
searchFullcallback referencesonKeywordChangeandsearchbut they are not included in the dependency array. This could lead to stale closures if these functions change.const searchFull = useCallback( async (keyword: string) => { // ... }, - [initialized, siteData.base, page.lang] + [initialized, siteData.base, page.lang, onKeywordChange, search] );theme/components/PostList/index.tsx (1)
1-9: LGTM!The import reorganization aligns with the PR's migration to
@rspress/core/theme-originaland properly uses the localLinkCardcomponent.theme/index.tsx (1)
1-11: LGTM!The export structure correctly exposes
DocLayout,Card, andLinkCardfrom local implementations, with the wildcard re-export from@rspress/core/theme-originalproviding base theme exports. Local named exports after the wildcard correctly shadow any conflicting names from the re-export.theme/utils/download.ts (1)
48-56: Cross-origin downloads may silently fail.For cross-origin URLs, the
downloadattribute is often ignored by browsers due to security restrictions, causing a navigation instead of a download. Consider adding a note or warning to users, or opening in a new tab.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @package.json:
- Line 5: The package.json dependency entry for "@alauda/doom" uses a
non-existent version "^1.17.6"; update the dependency to a valid version (e.g.,
change "@alauda/doom": "^1.17.6" to the correct published version such as
"1.7.1" or the intended "1.7.6"), or correct the package name if that was the
mistake, then run npm view @alauda/doom versions to confirm the chosen version
exists and npm install to verify resolution.
In @styles/index.css:
- Around line 21-35: .paginationItem declares font-weight twice; remove the
redundant declaration so only one font-weight assignment remains. In the
.paginationItem rule, delete the first font-weight: var(--font-weight-medium)
(the standalone occurrence) and keep the later --tw-font-weight and font-weight
pair (or vice versa) so the class has a single clear font-weight definition.
In @theme/layout/index.tsx:
- Around line 77-93: The anchor currently forces href="#" which can break
UX/accessibility; change the rendered element in the CustomMDXComponent.a block
to use the original props.href (e.g., href={props.href}) instead of "#" so the
link remains valid for screen readers and fallback behavior, keep the existing
preventDefault()/stopPropagation() in the onClick handler and the download prop
and preserve calling downloadFile(props.href, ...), and optionally ensure
aria-label/role is set if the link needs a button-like announcement.
In @theme/utils/download.ts:
- Around line 24-31: getBasePath currently falls back to a hardcoded
"/knowledge/" which can drift from the build config; update getBasePath to
derive its fallback from the project's configuration instead of a literal string
by importing the exported base path (or a BUILD_BASE_PATH env var) from the
build config (e.g., the value exported in doom.config.ts) and using that as the
fallback; ensure getBasePath still attempts the pathname regex first, then
returns the imported/configured base path if the regex fails.
🧹 Nitpick comments (5)
styles/doc-layout.css (1)
63-79: Sidebar usesoverflow: auto scrollwhich may cause unexpected behavior.The
overflow: auto scrolldeclaration setsoverflow-x: autoandoverflow-y: scroll. If the intent is to always show vertical scrollbar while auto-hiding horizontal, this is fine. However,scrollforces a scrollbar even when content fits, which may not be the desired UX.Consider using `overflow-y: auto` if scrollbar should only appear when needed
.rp-doc-layout__sidebar { ... - overflow: auto scroll; + overflow-x: auto; + overflow-y: auto; }styles/index.css (1)
153-165: Potential z-index conflict with doc-layout.css.The
.rp-doc-layout__menuhere usesz-index: 100(line 159), butdoc-layout.cssdefines--rp-z-index-nav-screen: 70as the highest z-index token. This hardcoded value bypasses the z-index hierarchy established in the CSS variables.Consider using the CSS variable system for consistency
.rp-doc-layout__menu { display: none; position: fixed; top: 0; left: 0; right: 0; - z-index: 100; + z-index: var(--rp-z-index-nav-screen); ... }Or define a new token in doc-layout.css if 100 is intentionally higher than nav-screen.
theme/layout/index.tsx (2)
69-95: Performance:getCustomMDXComponent()called on every anchor render.
getCustomMDXComponent()is invoked inside theacomponent, meaning it's called for every anchor element rendered in MDX content. Consider hoisting this call outside the component function or memoizing it.Hoist getCustomMDXComponent call
+const CustomMDXComponent = getCustomMDXComponent(); + export default () => { useEffect(() => { window.parent.postMessage(window.location.href, "*"); }, []); return ( <Layout ... components={{ a: (props: any) => { - const CustomMDXComponent = getCustomMDXComponent(); const pathname = getPathname(props.href); if (!shouldDownload(pathname)) { return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; } ... }, }} ></Layout> ); };
82-85: UseReact.MouseEventinstead ofEventfor type safety.The event parameter type
Event(line 82) should beReact.MouseEvent<HTMLAnchorElement>for proper TypeScript type checking in React. Additionally,stopImmediatePropagationis not a standard React synthetic event method.Fix event type
- onClick={(e: Event) => { + onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { e.preventDefault(); e.stopPropagation(); - e.stopImmediatePropagation?.(); downloadFile( props.href, pathname.split("/").pop() || "download" ); - return false; }}theme/utils/download.ts (1)
47-51: Empty catch block silently swallows URL parsing errors.The empty catch block provides no feedback when URL parsing fails. Consider logging a warning for debugging purposes.
Add minimal logging
try { path = new URL(url).pathname; } catch { + // URL parsing failed, use url as-is }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonstyles/doc-layout.cssstyles/index.csstheme/components/Card/index.tsxtheme/components/Checkbox/index.tsxtheme/layout/DocLayout/index.tsxtheme/layout/index.tsxtheme/utils/download.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- theme/components/Card/index.tsx
- theme/components/Checkbox/index.tsx
- theme/layout/DocLayout/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
theme/layout/index.tsx (1)
theme/utils/download.ts (3)
getPathname(16-22)shouldDownload(11-14)downloadFile(39-81)
🪛 Biome (2.1.2)
styles/index.css
[error] 28-28: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (6)
styles/doc-layout.css (3)
1-28: Well-structured CSS variables for layout tokens.The root CSS variables establish a clear z-index hierarchy and dimension tokens. The calculated margins for sidebar and outline using
max()andcalc()are a good approach for centering content.
179-205: Mobile sidebar transition and visibility handling looks good.The mobile breakpoint (≤768px) properly handles the sidebar with opacity, transform, and fixed positioning for a slide-in effect. The
--openmodifier correctly overrides visibility and positioning.
30-34: Media query syntax is widely supported—no changes needed.The
width >= 1280pxandwidth <= 1279pxsyntax uses CSS Media Queries Level 4 range syntax, which is fully supported across all modern browsers (Chrome/Chromium 104+, Edge 104+, Firefox, Safari 16.4+, and iOS Safari 16.4+). The boundary definitions correctly avoid overlap.styles/index.css (1)
62-109: Card and LinkCard styles are well-defined.The component styles follow consistent patterns with proper use of CSS variables for theming and appropriate transition properties for interactive states.
theme/utils/download.ts (2)
1-9: Extension list may need expansion for common file types.The
DOWNLOAD_EXTENSIONSlist includes code/data files but omits common document types like.doc,.xlsx,.csv. Verify if this is intentional based on the documentation content.
39-71: Same-origin download flow is well-implemented.The blob-based download approach for same-origin files correctly handles base path normalization, proper cleanup of blob URLs and DOM elements, and includes error handling with a console warning.
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
theme/components/HomeBanner/index.tsx (1)
14-19: Consider removing commented-out code.The commented-out
pageDataand logo logic appears to be obsolete following the migration to i18n-based content. Consider removing this dead code to improve readability, or add a TODO comment if it's planned for future implementation.theme/layout/HomeLayout/index.tsx (1)
1-6: Consider reordering imports for consistency.The
Reactimport on line 6 is placed after local imports. Standard convention places third-party imports before local/relative imports. Additionally, with React 17+ automatic JSX transform, this explicit import may be unnecessary unless the project specifically requires it.Suggested import order
-import { usePageData } from "@rspress/core/runtime"; -import { ReactNode } from "react"; - -import { HomeBanner } from "../../components/HomeBanner"; -import { HomeContent } from "../../components/HomeContent"; -import React from "react"; +import React, { ReactNode } from "react"; + +import { usePageData } from "@rspress/core/runtime"; + +import { HomeBanner } from "../../components/HomeBanner"; +import { HomeContent } from "../../components/HomeContent";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.jsontheme/components/HomeBanner/index.tsxtheme/layout/HomeLayout/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (4)
theme/components/HomeBanner/index.tsx (2)
11-31: LGTM! Good use of typed i18n hook.The generic type parameter
useI18n<typeof import("i18n")>()provides proper TypeScript inference for translation keys, improving type safety and enabling IDE autocompletion fort("knowledge_title")andt("knowledge_description").
1-1: Import path migration is correct.The import source
@rspress/core/runtimeis the established pattern across the entire codebase with no lingering old imports.theme/layout/HomeLayout/index.tsx (2)
24-28: LGTM!The footer refactor using semantic BEM-style class names (
home-footer,home-footer__content) improves maintainability and aligns with the centralized styling approach introduced instyles/index.css.
1-1: Import path change is correct and consistent across the codebase.The switch from
@rspress/runtimeto@rspress/core/runtimeis verified. All theme files consistently use@rspress/core/runtimewith no remaining uses of the old import path, confirming the migration is complete and the new path is properly exported from the upgraded package.
| @@ -0,0 +1,205 @@ | |||
| :root { | |||
There was a problem hiding this comment.
暂时没考虑预处理器，主要现在还是基本按官方指南来做的没试过加上预处理器
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| e.stopImmediatePropagation?.(); | ||
| downloadFile( |
There was a problem hiding this comment.
为啥不直接用原始的 <a download> 来做？可能用到的相关 API: withBase(), className="rp-link".
There was a problem hiding this comment.
可能跟我拿来测试的文件类型有关系，我之前尝试刚才也再试了下 , 浏览器可能吧 ipynb 文件当成可预览类型了，还是直接跳转到文件预览页面了，所以这里不直接用 download 了
There was a problem hiding this comment.
这里不要用 rspress 提供的链接组件，它拦截了 onClick，导致处理不了 download。
|
@yangxiaolang 再升一下 1.18.2，之前的版本 SSG 有问题 |
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @theme/layout/index.tsx:
- Around line 2-7: Update the dependency entry for @alauda/doom in package.json
from "^1.18.1" to "1.18.2" to pick up the SSG fixes; after changing the version,
run your package manager (npm install or yarn install) to update node_modules
and regenerate the lockfile (package-lock.json or yarn.lock) so the new version
is recorded, and ensure any build/SSG steps pass locally; reference the package
name "@alauda/doom" to locate the exact field to edit.
🧹 Nitpick comments (2)
styles/index.css (1)
36-48: Consider consolidating duplicate styles.The
:hoverand.activestates have identical declarations, andcursor: pointeris already defined in the base.paginationItemrule.♻️ Suggested consolidation
-.paginationItem:hover { - background-color: var(--rp-c-bg-soft); - cursor: pointer; - color: var(--rp-c-brand-dark); - border-radius: var(--rp-radius); -} - -.paginationItem.active { - background-color: var(--rp-c-bg-soft); - cursor: pointer; - color: var(--rp-c-brand-dark); - border-radius: var(--rp-radius); -} +.paginationItem:hover, +.paginationItem.active { + background-color: var(--rp-c-bg-soft); + color: var(--rp-c-brand-dark); + border-radius: var(--rp-radius); +}theme/layout/index.tsx (1)
72-99: Consider refactoring the custom anchor component for performance and maintainability.A few improvements to consider:
Performance:
getCustomMDXComponent()is called on every anchor render. Consider memoizing or lifting it outside the render function.Type safety: Use
React.MouseEvent<HTMLAnchorElement>instead ofEventfor proper typing.DRY: The filename extraction
pathname.split("/").pop() || "download"is duplicated.Cleanup:
return falseis unnecessary aftere.preventDefault()in React event handlers.♻️ Suggested refactor
+const CustomMDXComponent = getCustomMDXComponent(); + export default () => { const { siteData } = usePageData(); const base = siteData.base; // ... components={{ a: (props: any) => { - const CustomMDXComponent = getCustomMDXComponent(); const pathname = getPathname(props.href); if (!shouldDownload(pathname)) { return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; } + const filename = pathname.split("/").pop() || "download"; return ( <CustomMDXComponent.a {...props} href="#" - download={pathname.split("/").pop() || "download"} - onClick={(e: Event) => { + download={filename} + onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { e.preventDefault(); e.stopPropagation(); e.stopImmediatePropagation?.(); - downloadFile( - props.href, - pathname.split("/").pop() || "download", - base - ); - return false; + downloadFile(props.href, filename, base); }} ></CustomMDXComponent.a> ); }, }}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
styles/index.csstheme/layout/index.tsxtheme/utils/download.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- theme/utils/download.ts
🧰 Additional context used
🧬 Code graph analysis (1)
theme/layout/index.tsx (2)
theme/layout/HomeLayout/index.tsx (1)
usePageData(16-31)theme/utils/download.ts (3)
getPathname(16-22)shouldDownload(11-14)downloadFile(39-80)
🔇 Additional comments (10)
styles/index.css (8)
1-6: LGTM!The imports and root variable setup look correct. The
!importanton--rp-nav-heightis acceptable for overriding framework defaults.
8-19: LGTM!Clean link styling with appropriate transitions and hover states using CSS variables.
50-59: LGTM!The disabled state correctly removes interactivity, and the utility class is straightforward.
61-78: LGTM!Well-structured card component with consistent use of CSS variables for theming.
80-107: LGTM!Good use of explicit transition properties for performance, and consistent theming with CSS variables.
109-125: LGTM!Clean responsive styling for post metadata and simple breadcrumb spacing.
127-150: LGTM!Footer styling is well-structured with appropriate responsive behavior.
152-198: LGTM!Well-implemented responsive menu with appropriate fixed positioning, z-index stacking, and interactive button states.
theme/layout/index.tsx (2)
2-8: Import changes look appropriate.The switch to
@rspress/core/theme-originaland addition ofgetCustomMDXComponentaligns with the custom MDX anchor rendering pattern introduced below.
49-50: LGTM!Clean extraction of
basefromsiteDatafor download path normalization.
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
Summary by CodeRabbit
New Features
Style / Theming
Chores
✏️ Tip: You can customize this high-level summary in your review settings.