BA-1851: Add Missing MDX Doc Files for BaseApp Frontend Packages - Group 2 #168
Conversation
joaosoutto
requested review from
priscilladeroode,
anicioalexandre,
deboracosilveira and
ficast
|
WalkthroughThis pull request introduces comprehensive Storybook documentation for multiple components across the design system and navigation modules. The documentation provides detailed insights into the purpose, behavior, props, and usage of components such as Changes
Possibly Related PRs
Suggested Reviewers
Poem
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 (
|
![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 (17)
packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx (2)
24-24: Enhance props documentation with specific CircularProgressProps detailsThe props section could be more helpful by detailing the commonly used CircularProgressProps options such as
size,color, andthickness. This would save developers from having to look up the MUI documentation.Add details like:
- **CircularProgressProps** (MUI CircularProgress Props): Optional props passed directly to the underlying MUI CircularProgress component. + **CircularProgressProps** (MUI CircularProgress Props): Optional props passed directly to the underlying MUI CircularProgress component. + - size (number): The size of the circle (default: 40) + - color ('primary' | 'secondary' | string): The color of the component + - thickness (number): The thickness of the circle (default: 3.6) + - variant ('determinate' | 'indeterminate'): The variant to use (default: 'indeterminate')
35-43: Expand example usage with common scenariosWhile the current example is good, it would be helpful to show more real-world usage scenarios.
Add examples like:
```javascript import { LoadingState } from '@baseapp-frontend/design-system' const MyComponent = () => { - return <LoadingState CircularProgressProps={{ size: 40 }} /> + // Example 1: Basic usage + return <LoadingState /> + + // Example 2: Custom size and color + return <LoadingState CircularProgressProps={{ + size: 60, + color: 'secondary', + thickness: 4 + }} /> + + // Example 3: Usage within a container + return ( + <div style={{ height: '100vh', display: 'flex' }}> + <LoadingState CircularProgressProps={{ size: 40 }} /> + </div> + ) } export default MyComponent</blockquote></details> <details> <summary>packages/design-system/components/dialogs/Dialog/__storybook__/Dialog.mdx (3)</summary><blockquote> `10-10`: **Improve sentence structure in expected behavior** The sentence structure could be improved for clarity. ```diff - **Expected Behavior**: Opens as a modal overlay centered on the screen. Can be closed via a close button, clicking outside (if enabled), or pressing ESC key (if enabled). Should trap focus within the dialog when open for accessibility. + **Expected Behavior**: The dialog opens as a modal overlay centered on the screen. It can be closed via a close button, clicking outside (if enabled), or pressing the ESC key (if enabled). Focus is trapped within the dialog when open for accessibility.🧰 Tools
🪛 LanguageTool
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...(MISSING_IT_THERE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...cking outside (if enabled), or pressing ESC key (if enabled). Should trap focus wit...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
23-25: Expand props documentation with common Dialog propsThe props section should include more commonly used MUI Dialog props to provide a complete reference.
- **children** (ReactNode): The content to be displayed inside the dialog. - **open** (boolean): Controls whether the dialog is visible. - **onClose** ((event: {}, reason: "backdropClick" | "escapeKeyDown") => void): Callback fired when the dialog requests to be closed. + - **maxWidth** ('xs' | 'sm' | 'md' | 'lg' | 'xl' | false): Determine the max-width of the dialog. The dialog width grows with the size of the screen. + - **fullWidth** (boolean): If true, the dialog stretches to maxWidth. + - **disableEscapeKeyDown** (boolean): If true, hitting escape will not fire the onClose callback. + - **disableBackdropClick** (boolean): If true, clicking the backdrop will not fire the onClose callback.
36-49: Enhance example with common Dialog patternsThe example could demonstrate more common Dialog usage patterns.
```javascript import { Dialog } from '@baseapp-frontend/design-system' const MyComponent = () => { const [open, setOpen] = useState(false) + const handleClose = (event, reason) => { + if (reason !== 'backdropClick') { + setOpen(false) + } + } + return ( - <Dialog open={open} onClose={() => setOpen(false)}> - <div>Dialog content</div> - </Dialog> + <> + <button onClick={() => setOpen(true)}>Open Dialog</button> + + <Dialog + open={open} + onClose={handleClose} + maxWidth="sm" + fullWidth + disableBackdropClick + > + <DialogTitle>Dialog Title</DialogTitle> + <DialogContent> + <DialogContentText> + Dialog content with text and other components. + </DialogContentText> + </DialogContent> + <DialogActions> + <Button onClick={() => setOpen(false)}>Cancel</Button> + <Button onClick={() => setOpen(false)} color="primary"> + Confirm + </Button> + </DialogActions> + </Dialog> + </> ) } export default MyComponent</blockquote></details> <details> <summary>packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx (3)</summary><blockquote> `20-20`: **Fix grammar in use cases section** Add the missing article "a" for better readability. ```diff - Content-focused websites where navigation clarity is priority + Content-focused websites where navigation clarity is a priority🧰 Tools
🪛 LanguageTool
[uncategorized] ~20-~20: You might be missing the article “a” here.
Context: ...ed websites where navigation clarity is priority ## Props - navData (array): Navig...(AI_EN_LECTOR_MISSING_DETERMINER_A)
24-26: Enhance props documentation with detailed type informationThe props section should provide more detailed type information and include additional customization props.
- **navData** (array): Navigation items data containing routes, icons, and labels + **navData** (Array<{ + title: string, + path?: string, + icon?: ReactNode, + info?: ReactNode, + children?: Array<{ + title: string, + path: string + }> + }>): Navigation items data containing routes, icons, and labels - **openNav** (boolean): Whether the navigation is open or closed - **onCloseNav** (function): Callback function to close the navigation + - **logo** (ReactNode): Custom logo component to display + - **actions** (ReactNode): Additional actions to display in the navigation bar + - **disableAccount** (boolean): Whether to hide the account menu
38-57: Improve example with proper imports and realistic configurationThe example should include proper imports and show a more realistic navigation configuration.
```javascript -import { NavCentered } from '@baseapp-frontend/design-system' +import { NavCentered } from '@baseapp-frontend/design-system' +import HomeIcon from '@mui/icons-material/Home' +import InfoIcon from '@mui/icons-material/Info' +import ContactIcon from '@mui/icons-material/ContactMail' +import { Logo } from '../components/Logo' const MyComponent = () => { const [openNav, setOpenNav] = useState(false) + + const navConfig = [ + { + title: 'Home', + path: '/', + icon: <HomeIcon />, + }, + { + title: 'About', + path: '/about', + icon: <InfoIcon />, + children: [ + { title: 'Team', path: '/about/team' }, + { title: 'Career', path: '/about/career' }, + ] + }, + { + title: 'Contact', + path: '/contact', + icon: <ContactIcon /> + } + ] + return ( <NavCentered - navData={[ - { title: 'Home', icon: <MenuIcon /> }, - { title: 'About', icon: <InfoIcon /> }, - { title: 'Contact', icon: <ContactIcon /> }, - ]} + logo={<Logo />} + navData={navConfig} openNav={openNav} onCloseNav={() => setOpenNav(false)} + disableAccount={false} /> ) }packages/components/modules/navigations/Header/__storybook__/Header.mdx (1)
42-56: Improve example code completeness and importsThe example code could be enhanced in several ways:
- Use more specific imports instead of importing everything from design-system
- Include essential props mentioned in the props section
- Add comments explaining the purpose of each prop
-import { Header } from '@baseapp-frontend/design-system' -import { LogoIcon } from '@baseapp-frontend/design-system' -import { AccountMenu } from '@baseapp-frontend/design-system' +import { Header } from '@baseapp-frontend/design-system/components/Header' +import { LogoIcon } from '@baseapp-frontend/design-system/components/Logo' +import { AccountMenu } from '@baseapp-frontend/design-system/components/AccountMenu' const MyComponent = () => { + // Handle navigation menu toggle + const handleOpenNav = () => { + // Implementation + } + return ( <Header settings={{ themeLayout: 'horizontal' }} + onOpenNav={handleOpenNav} + ToolbarProps={{ + sx: { height: 64 } + }} > <LogoIcon /> <AccountMenu /> </Header> ) }packages/components/modules/navigations/NavVertical/__storybook__/NavVertical.mdx (1)
41-62: Enhance example code with proper imports and state managementThe example code needs improvements:
- Missing imports for icons
- State management could follow better practices
- Lacks error handling
+import { HomeIcon, InfoIcon, ContactIcon } from '@baseapp-frontend/design-system/icons' import { NavVertical } from '@baseapp-frontend/design-system' -import { BaseAppLogoCondensed } from '@baseapp-frontend/design-system' const MyComponent = () => { - const [openNav, setOpenNav] = useState(false) + // Use callback to prevent unnecessary re-renders + const [openNav, setOpenNav] = useState(false) + const handleCloseNav = useCallback(() => { + setOpenNav(false) + }, []) return ( <NavVertical navData={[ - { title: 'Home', icon: <MenuIcon /> }, - { title: 'About', icon: <InfoIcon /> }, - { title: 'Contact', icon: <ContactIcon /> }, + { title: 'Home', path: '/', icon: <HomeIcon /> }, + { title: 'About', path: '/about', icon: <InfoIcon /> }, + { title: 'Contact', path: '/contact', icon: <ContactIcon /> } ]} openNav={openNav} - onCloseNav={() => setOpenNav(false)} + onCloseNav={handleCloseNav} hideToggleButton={true} /> ) }packages/components/modules/navigations/NavigationLayout/__storybook__/NavigationLayout.mdx (2)
42-71: Improve example code with proper theme handling and settings managementThe example needs several improvements:
- Proper theme import and usage
- Meaningful settings management
- Better type safety
import { NavigationLayout } from '@baseapp-frontend/design-system' import { BaseAppLogoCondensed } from '@baseapp-frontend/design-system' +import { defaultTheme } from '@baseapp-frontend/design-system/theme' +import { useState } from 'react' const MyComponent = () => { + const [settings, setSettings] = useState(defaultTheme.settings) + + const handleSettingsChange = useCallback((newSettings) => { + setSettings(prev => ({ + ...prev, + ...newSettings + })) + }, []) return ( <NavigationLayout navData={[ { subheader: 'BASEAPP', items: [{ title: 'Home', path: '/', icon: <HomeIcon /> }], }, // ... other items ]} - settings={defaultTheme.settings} - setSettings={() => {}} + settings={settings} + setSettings={handleSettingsChange} LogoIcon={BaseAppLogoCondensed} /> ) }
1-1: Maintain consistency across documentation filesWhile the documentation provides good coverage, consider maintaining consistency across all MDX files by:
- Using the same format for type definitions
- Following the same pattern for examples (imports, state management, etc.)
- Providing similar levels of detail for common props (e.g., settings, LogoIcon)
packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx (3)
9-10: Consider adding responsive breakpoint details.The documentation would be more helpful if it specified the breakpoints at which the component switches between desktop and mobile views.
24-29: Enhance props documentation with types and required status.Consider enhancing the props documentation by:
- Adding TypeScript types for each prop
- Indicating which props are required vs optional
- Including default values where applicable
Example format:
- **navData** (array, required): Navigation items data containing routes, icons, and labels - Type: `Array<{ subheader: string, items: Array<{ title: string, icon: ReactNode }> }>`
41-73: Consider adding TypeScript example.The example is comprehensive but could be more helpful with TypeScript types. Consider adding a TypeScript version of the example to demonstrate proper typing.
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/AccountPopover.mdx (2)
9-10: Add accessibility considerations.Consider documenting accessibility features such as:
- Keyboard navigation support
- ARIA attributes
- Screen reader behavior
45-77: Enhance example with realistic scenarios.Consider improving the example by:
- Adding a more realistic user profile object
- Demonstrating error handling
- Showing integration with authentication
- Including examples of custom styling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/AccountPopover.mdx(1 hunks)packages/components/modules/navigations/Header/__storybook__/Header.mdx(1 hunks)packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx(1 hunks)packages/components/modules/navigations/NavHorizontal/__storybook__/NavHorizontal.mdx(1 hunks)packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx(1 hunks)packages/components/modules/navigations/NavVertical/__storybook__/NavVertical.mdx(1 hunks)packages/components/modules/navigations/NavigationLayout/__storybook__/NavigationLayout.mdx(1 hunks)packages/design-system/components/Scrollbar/__storybook__/Scrollbar.mdx(1 hunks)packages/design-system/components/dialogs/Dialog/__storybook__/Dialog.mdx(1 hunks)packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/design-system/components/Scrollbar/storybook/Scrollbar.mdx
- packages/components/modules/navigations/NavHorizontal/storybook/NavHorizontal.mdx
🧰 Additional context used
🪛 LanguageTool
packages/design-system/components/dialogs/Dialog/__storybook__/Dialog.mdx
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...
(MISSING_IT_THERE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...cking outside (if enabled), or pressing ESC key (if enabled). Should trap focus wit...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx
[uncategorized] ~20-~20: You might be missing the article “a” here.
Context: ...ed websites where navigation clarity is priority ## Props - navData (array): Navig...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🔇 Additional comments (1)
packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx (1)
1-3: LGTM! Title follows Storybook naming conventions.
| - **settings** (object): Configuration object for theme and layout settings | ||
| - **themeLayout**: Determines the header layout ('horizontal' | 'centered' | 'vertical') | ||
| - **children** (ReactNode): Optional content to render within the header | ||
| - **onOpenNav** (function): Callback function for handling navigation menu toggle | ||
| - **LogoIcon** (component): Custom logo component to display in the header | ||
| - **AccountMenu** (component): Custom account menu component, defaults to DefaultAccountMenu | ||
| - **AccountMenuProps** (object): Props passed to the AccountMenu component | ||
| - **ToolbarProps** (object): Props passed to the underlying MUI Toolbar component |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance props documentation with type information and required/optional status
The props section should clearly indicate:
- Type information for each prop (e.g.,
themeLayoutis shown but others aren't) - Which props are required vs optional
- Default values where applicable
Example format:
- **settings** (object, required): Configuration object for theme and layout settings
- **themeLayout** ('horizontal' | 'centered' | 'vertical'): Determines the header layout
- Default: { themeLayout: 'horizontal' }| - **navData** (NavigationData): Navigation configuration data containing routes, icons, and menu structure | ||
| - **settings** (UISettings): UI configuration settings for the navigation component | ||
| - **LogoIcon** (React.FC): Optional custom logo component to display at the top | ||
| - **openNav** (boolean): Controls the open/closed state of the navigation drawer | ||
| - **onCloseNav** (function): Callback function when navigation drawer is closed | ||
| - **hideToggleButton** (boolean): Whether to hide the toggle button for the navigation drawer |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document complex type definitions
The props section references complex types (NavigationData, UISettings) without explaining their structure. Consider adding:
- Type definitions with examples
- Links to relevant type documentation
Example addition:
- **navData** (NavigationData): Navigation configuration data containing routes, icons, and menu structure
```typescript
type NavigationData = {
title: string;
path?: string;
icon?: React.ReactNode;
info?: React.ReactNode;
children?: NavigationData[];
// ... other properties
}
<!-- This is an auto-generated comment by CodeRabbit -->
| - **navData** (NavigationData): Navigation configuration data containing routes, icons, and menu structure | ||
| - **settings** (UISettings): UI configuration settings for the navigation components | ||
| - **setSettings** (function): Callback to update UI settings | ||
| - **LogoIcon** (React.FC): Optional custom logo component | ||
| - **AccountMenu** (React.FC): Optional custom account menu component | ||
| - **AccountMenuProps** (object): Optional props for the account menu component | ||
| - **ToolbarProps** (MuiToolbarProps): Optional props for the Material-UI Toolbar component | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document settings structure and callback usage
The props documentation needs enhancement:
- Add structure of UISettings type
- Explain proper usage of setSettings callback
- Document ToolbarProps interface
Example addition:
- **settings** (UISettings): UI configuration settings
```typescript
interface UISettings {
themeMode: 'light' | 'dark';
themeLayout: 'vertical' | 'horizontal' | 'mini';
// ... other properties
}- setSettings (function): Callback to update UI settings
(settings: Partial<UISettings>) => void
<!-- This is an auto-generated comment by CodeRabbit -->
| - **MenuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action | ||
| - **PopoverStyles** (object): Styles for the popover component | ||
| - **CurrentUser** (object): The current user object containing profile information | ||
| - **SwitchProfileMenu** (object): The switch profile menu object containing label and onClick handlers for each action | ||
| - **SwitchProfileMenuProps** (object): The switch profile menu props object containing label and onClick handlers for each action | ||
| - **ProfilesList** (object): The profiles list object containing label and onClick handlers for each action | ||
| - **ProfilesListProps** (object): The profiles list props object containing label and onClick handlers for each action | ||
| - **AddProfileMenuItem** (object): The add profile menu item object containing label and onClick handlers for each action | ||
| - **AddProfileMenuItemProps** (object): The add profile menu item props object containing label and onClick handlers for each action | ||
| - **LogoutItemProps** (object): The logout menu item props object containing label and onClick handlers for each action | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify props structure to reduce duplication.
The current props structure has several overlapping props with similar purposes (e.g., SwitchProfileMenu/SwitchProfileMenuProps, ProfilesList/ProfilesListProps). Consider simplifying by:
- Consolidating duplicate props
- Using consistent naming (remove redundant "Props" suffix)
- Organizing related props into logical groups
Example structure:
interface AccountPopoverProps {
menuItems: MenuItem[];
switchProfile: {
label: string;
onClick: () => void;
};
profiles: {
list: Profile[];
onAdd: () => void;
};
onLogout: () => void;
styles?: PopoverStyles;
}
|
please review your use cases, specially the Current Usage, to have a simple and direct info about the usage inside our package |
There was a problem hiding this comment.
plase update the title of all mdx. They all include "@baseapp-frontend-template" which is wrong, since we are talking about the packages. Besides that, the idea is that document pages live close to the referring component, like on the image.
To do so, use the same name of the stories, eg @baseapp-frontend | designSystem/Avatars/AvatarWithPlaceholder
|
|
||
| ## Use Cases | ||
|
|
||
| - **Current Usage**: Used as the main navigation header in applications requiring a centered, horizontal navigation layout. |
There was a problem hiding this comment.
this definition is a bit weird, just keep it simple and explain that is one option of navigation, etc
|
|
||
| ## Use Cases | ||
|
|
||
| - **Current Usage**: Used as the main navigation header in applications requiring a horizontal navigation layout. |
There was a problem hiding this comment.
i think you can remove this 'header' and also does not mention in applications requiring from the defs, bec this layout can also be one option so the same app can offer multiple nav options
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
packages/design-system/components/displays/__storybook__/Displays.mdx (2)
26-26: Enhance props documentation with common CircularProgress options.The props section could be more helpful by listing commonly used CircularProgress props with their types and default values (e.g., size, thickness, color).
Consider expanding the props documentation like this:
- **CircularProgressProps** (MUI CircularProgress Props): Optional props passed directly to the underlying MUI CircularProgress component. + **CircularProgressProps** (MUI CircularProgress Props): Optional props passed directly to the underlying MUI CircularProgress component: + - `size` (number, default: 40): The size of the circle in pixels + - `thickness` (number, default: 3.6): The thickness of the circle + - `color` ('primary' | 'secondary' | 'inherit', default: 'primary'): The color of the component
37-45: Add a real-world usage example.While the current example is good for basic usage, consider adding a more practical example showing integration with data loading.
Consider adding this example:
const DataList = () => { const [isLoading, setIsLoading] = useState(true) const [data, setData] = useState([]) useEffect(() => { fetchData() .then(response => setData(response)) .finally(() => setIsLoading(false)) }, []) if (isLoading) { return <LoadingState CircularProgressProps={{ size: 40, color: 'primary' }} /> } return <List items={data} /> }packages/design-system/components/Scrollbar/__storybook__/Scrollbar.mdx (1)
26-28: Document additional Scrollbar customization props.The props section should include all available customization options and event handlers.
Consider expanding the props documentation:
- **children** (ReactNode): The content to be wrapped with the custom scrollbar. - **sx** (SxProps): Optional Material-UI system props for additional styling. + **children** (ReactNode): The content to be wrapped with the custom scrollbar. + **sx** (SxProps): Optional Material-UI system props for additional styling. + **ref** (Ref): Forward ref to access the scrollbar container. + **onScroll** (function): Callback fired when scrolling occurs. + **autoHide** (boolean, default: true): Whether to hide scrollbar when not scrolling. + **timeout** (number, default: 1000): Delay before hiding scrollbar.packages/design-system/components/dialogs/__storybook__/Dialogs.mdx (3)
14-14: Improve sentence structure in Expected Behavior.The sentence is grammatically incomplete.
- Opens as a modal overlay centered on the screen. Can be closed via a close button, clicking outside (if enabled), or pressing ESC key (if enabled). + The dialog opens as a modal overlay centered on the screen. It can be closed via a close button, clicking outside (if enabled), or pressing ESC key (if enabled).🧰 Tools
🪛 LanguageTool
[style] ~14-~14: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...(MISSING_IT_THERE)
27-29: Document additional Dialog props.The props section should include more commonly used MUI Dialog props.
Consider expanding the props documentation:
- **children** (ReactNode): The content to be displayed inside the dialog. - **open** (boolean): Controls whether the dialog is visible. - **onClose** ((event: {}, reason: "backdropClick" | "escapeKeyDown") => void): Callback fired when the dialog requests to be closed. + **children** (ReactNode): The content to be displayed inside the dialog. + **open** (boolean): Controls whether the dialog is visible. + **onClose** ((event: {}, reason: "backdropClick" | "escapeKeyDown") => void): Callback fired when the dialog requests to be closed. + **maxWidth** ('xs' | 'sm' | 'md' | 'lg' | 'xl'): Determine the max-width of the dialog. + **fullWidth** (boolean): If true, the dialog stretches to maxWidth. + **disableEscapeKeyDown** (boolean): If true, hitting escape will not fire the onClose callback. + **disableBackdropClick** (boolean): If true, clicking the backdrop will not fire the onClose callback.
77-83: Add type information to ConfirmDialog props.The props section should include type information for better clarity.
- **open** (boolean): Controls the visibility of the dialog - **title** (string): The heading text displayed at the top of the dialog - **content** (ReactNode): The main message or content displayed in the dialog body - **cancelText** (string): Custom text for the cancel button (optional, defaults to "Cancel") - **onClose** (function): Callback function executed when the cancel button is clicked - **action** (ReactNode): The action button to be displayed in the dialog + **open** (boolean, required): Controls the visibility of the dialog + **title** (string, required): The heading text displayed at the top of the dialog + **content** (ReactNode, required): The main message or content displayed in the dialog body + **cancelText** (string, optional, default: "Cancel"): Custom text for the cancel button + **onClose** (() => void, required): Callback function executed when the cancel button is clicked + **action** (ReactNode, required): The action button to be displayed in the dialogpackages/components/modules/navigations/__storybook__/NavigationsModule.mdx (3)
32-39: Enhance props documentation with required/optional indicators.The props documentation would be more helpful if it indicated which props are required and which are optional. Consider using TypeScript-style notation (e.g.,
navData?: NavigationData).
50-79: Add error handling to the example code.The example code should demonstrate proper error handling for the
setSettingscallback.const MyComponent = () => { + const handleSettingsChange = (newSettings) => { + try { + // Apply new settings + setSettings(newSettings); + } catch (error) { + console.error('Failed to update settings:', error); + } + }; + return ( <NavigationLayout navData={[/* ... */]} settings={defaultTheme.settings} - setSettings={() => {}} + setSettings={handleSettingsChange} LogoIcon={BaseAppLogoCondensed} /> ) }
246-277: Enhance example code to be more production-ready.The example code sections could be improved in several ways:
- Add missing icon imports
- Show proper state management
- Include TypeScript types
Here's an improved example for NavMini (similar improvements apply to other components):
import { NavMini } from '@baseapp-frontend/design-system' -import { FavoriteIcon, MentionIcon, MenuIcon, PenEditIcon } from '@baseapp-frontend/design-system' +import { + FavoriteIcon, + MentionIcon, + MenuIcon, + PenEditIcon, + type NavItem, + type NavSettings +} from '@baseapp-frontend/design-system' import { BaseAppLogoCondensed } from '@baseapp-frontend/design-system' +import { useState } from 'react' -const MyComponent = () => { +const MyComponent: React.FC = () => { + const [isOpen, setIsOpen] = useState(false) + + const handleClose = () => { + setIsOpen(false) + console.log('NavMini closed') + } return ( <NavMini navData={[/* ... */]} settings={defaultTheme.settings} LogoIcon={BaseAppLogoCondensed} - openNav={false} + openNav={isOpen} hideToggleButton={true} - onCloseNav={() => console.log('NavMini closed')} + onCloseNav={handleClose} /> ) }Also applies to: 312-331, 369-390, 425-444
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/components/modules/navigations/__storybook__/NavigationsModule.mdx(1 hunks)packages/design-system/components/Scrollbar/__storybook__/Scrollbar.mdx(1 hunks)packages/design-system/components/dialogs/__storybook__/Dialogs.mdx(1 hunks)packages/design-system/components/displays/__storybook__/Displays.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/design-system/components/dialogs/__storybook__/Dialogs.mdx
[style] ~14-~14: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...
(MISSING_IT_THERE)
🔇 Additional comments (3)
packages/components/modules/navigations/__storybook__/NavigationsModule.mdx (3)
1-3: LGTM! Well-structured MDX documentation.The file follows Storybook's MDX documentation standards with proper imports and title structure.
1-444: LGTM! Well-maintained documentation consistency.The documentation follows a consistent structure across all components, making it easy to navigate and understand. Each section (Purpose, Expected Behavior, Use Cases, Props, Notes, Example Usage) is consistently formatted and provides valuable information.
1-444: Documentation successfully meets PR objectives.The MDX documentation is comprehensive, follows the provided template, and effectively documents all the required components (NavigationLayout, Header, AccountPopover, NavMini, NavCentered, NavVertical, NavHorizontal). This satisfies the PR objective of adding missing MDX documentation files.
| ## Components | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility guidelines section.
Consider adding an Accessibility section to the documentation that covers:
- ARIA attributes
- Keyboard navigation
- Screen reader compatibility
- Color contrast requirements
| - **MenuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action | ||
| - **PopoverStyles** (object): Styles for the popover component | ||
| - **CurrentUser** (object): The current user object containing profile information | ||
| - **SwitchProfileMenu** (object): The switch profile menu object containing label and onClick handlers for each action | ||
| - **SwitchProfileMenuProps** (object): The switch profile menu props object containing label and onClick handlers for each action | ||
| - **ProfilesList** (object): The profiles list object containing label and onClick handlers for each action | ||
| - **ProfilesListProps** (object): The profiles list props object containing label and onClick handlers for each action | ||
| - **AddProfileMenuItem** (object): The add profile menu item object containing label and onClick handlers for each action | ||
| - **AddProfileMenuItemProps** (object): The add profile menu item props object containing label and onClick handlers for each action | ||
| - **LogoutItemProps** (object): The logout menu item props object containing label and onClick handlers for each action | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify props documentation by removing redundancy.
Several props have redundant entries with "Props" suffix. Consider consolidating them and adding type information:
- **MenuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action
- **PopoverStyles** (object): Styles for the popover component
- **CurrentUser** (object): The current user object containing profile information
- **SwitchProfileMenu** (object): The switch profile menu object containing label and onClick handlers for each action
- **SwitchProfileMenuProps** (object): The switch profile menu props object containing label and onClick handlers for each action
+ **menuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action
+ **popoverStyles** (MUIPopoverProps): Material-UI Popover component props
+ **currentUser** (User): Current user profile information
+ **switchProfileMenu** ({
+ label: string;
+ onClick: () => void;
+ }): Configuration for the profile switching menuCommittable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/AccountPopover.mdx (1)
24-34: 🛠️ Refactor suggestionSimplify props structure to reduce duplication.
The props section contains several overlapping props with similar purposes.
As mentioned in the previous review, consider simplifying the props structure:
- **MenuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action - **PopoverStyles** (object): Styles for the popover component - **CurrentUser** (object): The current user object containing profile information - **SwitchProfileMenu** (object): The switch profile menu object containing label and onClick handlers for each action - **SwitchProfileMenuProps** (object): The switch profile menu props object containing label and onClick handlers for each action - **ProfilesList** (object): The profiles list object containing label and onClick handlers for each action - **ProfilesListProps** (object): The profiles list props object containing label and onClick handlers for each action - **AddProfileMenuItem** (object): The add profile menu item object containing label and onClick handlers for each action - **AddProfileMenuItemProps** (object): The add profile menu item props object containing label and onClick handlers for each action - **LogoutItemProps** (object): The logout menu item props object containing label and onClick handlers for each action + **menuItems** (MenuItem[]): Array of menu items containing label and onClick handlers for each action + **styles** (PopoverStyles): Optional styles for customizing the popover appearance + **currentUser** (User): The current user object containing profile information + **switchProfile** ({ + label: string; + onClick: () => void; + }): Configuration for the profile switching functionality + **profiles** ({ + list: Profile[]; + onAdd: () => void; + }): Profile list configuration and add profile handler + **onLogout** (() => void): Handler for logout action
🧹 Nitpick comments (8)
packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx (1)
14-14: Simplify the current usage descriptionThe current usage description is too technical. Keep it simple and focus on the component's role.
- - **Current Usage**: Used as a centered horizontal navigation component with responsive mobile drawer. + - **Current Usage**: Provides a centered navigation menu that adapts to both desktop and mobile views.packages/components/modules/navigations/Header/__storybook__/Header.mdx (1)
47-54: Add error handling to example usageThe example should demonstrate proper error handling for the onOpenNav callback.
const MyComponent = () => { + const handleOpenNav = React.useCallback((event: React.MouseEvent) => { + try { + // Add your navigation logic here + } catch (error) { + console.error('Failed to open navigation:', error); + } + }, []); + return ( <Header settings={{ themeLayout: 'horizontal' }}> <LogoIcon /> <AccountMenu /> + onOpenNav={handleOpenNav} </Header> ) }packages/components/modules/navigations/NavigationLayout/__storybook__/NavigationLayout.mdx (1)
64-64: Improve setSettings example implementationThe current example shows an empty callback. Demonstrate proper settings management.
- setSettings={() => {}} + setSettings={(newSettings) => { + // Example: Update settings in state/context + console.log('Updating settings:', newSettings); + // updateSettings(newSettings); + }}packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx (2)
24-30: Enhance props documentation with type information and default values.The props section should include more detailed type information and default values for better developer understanding.
Consider enhancing the props documentation like this:
- **navData** (array): Navigation items data containing routes and content + **navData** (NavigationItem[]): Navigation items data containing routes and content. Each item should conform to the NavigationItem interface with subheader and items properties. - **settings** (object): Configuration settings for the navigation component + **settings** (NavigationSettings): Configuration settings for the navigation component. Controls theme, behavior, and display options. - **setSettings** (function): Callback to update navigation settings + **setSettings** ((settings: NavigationSettings) => void): Callback to update navigation settings. - **LogoIcon** (component): Optional custom logo component to display at top + **LogoIcon** (React.ComponentType): Optional custom logo component to display at top. Defaults to undefined. - **openNav** (boolean): Controls visibility of mobile navigation drawer + **openNav** (boolean): Controls visibility of mobile navigation drawer. Defaults to false. - **onCloseNav** (function): Callback when mobile navigation drawer closes + **onCloseNav** (() => void): Callback when mobile navigation drawer closes. - **hideToggleButton** (boolean): Optional flag to hide the navigation toggle button, defaults to false + **hideToggleButton** (boolean): Optional flag to hide the navigation toggle button. Defaults to false.
44-76: Add TypeScript interfaces to example code.The example usage would be more helpful with TypeScript interfaces and proper type annotations.
Consider enhancing the example like this:
import { NavMini } from '@baseapp-frontend/design-system' import { FavoriteIcon, MentionIcon, MenuIcon, PenEditIcon } from '@baseapp-frontend/design-system' import { BaseAppLogoCondensed } from '@baseapp-frontend/design-system' + +interface NavigationItem { + subheader: string; + items: Array<{ + title: string; + icon: React.ReactNode; + }>; +} const MyComponent = () => { + const navData: NavigationItem[] = [ - return ( - <NavMini - navData={[packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx (3)
10-10: Improve grammar in Expected Behavior section.The sentence structure needs improvement for better clarity.
Consider revising the text:
- Opens as a modal overlay centered on the screen. Can be closed via a close button, clicking outside (if enabled), or pressing ESC key (if enabled). + The dialog opens as a modal overlay centered on the screen. It can be closed via a close button, clicking outside (if enabled), or pressing the ESC key (if enabled).🧰 Tools
🪛 LanguageTool
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...(MISSING_IT_THERE)
23-25: Enhance props documentation with additional properties.The props section is missing some commonly used properties of the Dialog component.
Consider adding these additional props:
- **children** (ReactNode): The content to be displayed inside the dialog. - **open** (boolean): Controls whether the dialog is visible. - **onClose** ((event: {}, reason: "backdropClick" | "escapeKeyDown") => void): Callback fired when the dialog requests to be closed. +- **maxWidth** ("xs" | "sm" | "md" | "lg" | "xl"): Determine the max-width of the dialog. Defaults to "sm". +- **fullWidth** (boolean): If true, the dialog stretches to maxWidth. Defaults to false. +- **fullScreen** (boolean): If true, the dialog will be displayed in full screen. Defaults to false. +- **disableEscapeKeyDown** (boolean): If true, hitting escape will not fire the onClose callback. Defaults to false. +- **disableBackdropClick** (boolean): If true, clicking the backdrop will not fire the onClose callback. Defaults to false.
36-49: Enhance example usage with additional features.The example usage could demonstrate more features of the Dialog component.
Consider enhancing the example:
import { Dialog } from '@baseapp-frontend/design-system' const MyComponent = () => { const [open, setOpen] = useState(false) return ( - <Dialog open={open} onClose={() => setOpen(false)}> - <div>Dialog content</div> + <Dialog + open={open} + onClose={() => setOpen(false)} + maxWidth="md" + fullWidth + disableBackdropClick={false} + disableEscapeKeyDown={false} + > + <DialogTitle>Example Dialog</DialogTitle> + <DialogContent> + <DialogContentText> + This is an example of a dialog with additional features. + </DialogContentText> + </DialogContent> + <DialogActions> + <Button onClick={() => setOpen(false)}>Cancel</Button> + <Button onClick={() => setOpen(false)} variant="contained"> + Confirm + </Button> + </DialogActions> </Dialog> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/AccountPopover.mdx(1 hunks)packages/components/modules/navigations/Header/__storybook__/Header.mdx(1 hunks)packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx(1 hunks)packages/components/modules/navigations/NavHorizontal/__storybook__/NavHorizontal.mdx(1 hunks)packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx(1 hunks)packages/components/modules/navigations/NavVertical/__storybook__/NavVertical.mdx(1 hunks)packages/components/modules/navigations/NavigationLayout/__storybook__/NavigationLayout.mdx(1 hunks)packages/design-system/components/Scrollbar/__storybook__/Scrollbar.mdx(1 hunks)packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx(1 hunks)packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/design-system/components/displays/LoadingState/storybook/LoadingState.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/components/Scrollbar/storybook/Scrollbar.mdx
🧰 Additional context used
🪛 LanguageTool
packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...
(MISSING_IT_THERE)
| - **navData** (array): Navigation items data containing routes and content | ||
| - **openNav** (boolean): Controls visibility of mobile navigation drawer | ||
| - **onCloseNav** (function): Callback when mobile navigation drawer closes |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance props documentation with type information
Add type information and required/optional status for each prop.
- **navData** (array): Navigation items data containing routes and content
- **openNav** (boolean): Controls visibility of mobile navigation drawer
- **onCloseNav** (function): Callback when mobile navigation drawer closes
+ **navData** (NavigationItem[], required): Array of navigation items containing routes and content
+ ```typescript
+ interface NavigationItem {
+ title: string;
+ icon?: React.ReactNode;
+ path?: string;
+ }
+ ```
+ **openNav** (boolean, required): Controls visibility of mobile navigation drawer
+ **onCloseNav** (() => void, required): Callback function invoked when mobile navigation drawer closes| - **settings** (object): Configuration settings for the navigation component | ||
| - **setSettings** (function): Callback to update navigation settings | ||
| - **navData** (array): Navigation items data containing routes and content |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document settings type structure
Add detailed type information for the settings object.
- **settings** (object): Configuration settings for the navigation component
- **setSettings** (function): Callback to update navigation settings
+ **settings** (UISettings, required): Configuration settings for the navigation component
+ ```typescript
+ interface UISettings {
+ themeMode: 'light' | 'dark';
+ themeLayout: 'vertical' | 'horizontal' | 'mini';
+ themeStretch: boolean;
+ themeContrast: 'default' | 'bold';
+ }
+ ```
+ **setSettings** ((settings: Partial<UISettings>) => void, required): Callback to update navigation settings|
|
||
| ## Props | ||
|
|
||
| - **navData** (NavigationData): Navigation configuration data containing routes, icons, and menu structure |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document NavigationData type structure
Add detailed type information for the NavigationData structure.
- **navData** (NavigationData): Navigation configuration data containing routes, icons, and menu structure
+ **navData** (NavigationData[], required): Navigation configuration data containing routes, icons, and menu structure
+ ```typescript
+ interface NavigationData {
+ subheader?: string;
+ items: Array<{
+ title: string;
+ path?: string;
+ icon?: React.ReactNode;
+ info?: React.ReactNode;
+ children?: NavigationData[];
+ }>;
+ }
+ ```| ```javascript | ||
| import { AccountPopover } from '@baseapp-frontend/design-system' | ||
|
|
||
| const MyComponent = () => { | ||
| return ( | ||
| <AccountPopover | ||
| MenuItemsProps={{ | ||
| menuItems: [ | ||
| { label: 'Profile', onClick: () => console.log('Profile clicked') }, | ||
| { label: 'Settings', onClick: () => console.log('Settings clicked') }, | ||
| ], | ||
| }} | ||
| SwitchProfileMenuProps={{ | ||
| label: 'Switch Profile', | ||
| onClick: () => console.log('Switch Profile clicked'), | ||
| }} | ||
| ProfilesListProps={{ | ||
| label: 'Profiles List', | ||
| onClick: () => console.log('Profiles List clicked'), | ||
| }} | ||
| AddProfileMenuItemProps={{ | ||
| label: 'Add Profile', | ||
| onClick: () => console.log('Add Profile clicked'), | ||
| }} | ||
| LogoutItemProps={{ | ||
| label: 'Logout', | ||
| onClick: () => console.log('Logout clicked'), | ||
| }} | ||
| /> | ||
| ) | ||
| } | ||
| export default MyComponent | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update example usage to match simplified props structure.
The example usage should reflect the simplified props structure for better clarity.
Consider updating the example:
const MyComponent = () => {
return (
<AccountPopover
- MenuItemsProps={{
- menuItems: [
+ menuItems={[
{ label: 'Profile', onClick: () => console.log('Profile clicked') },
{ label: 'Settings', onClick: () => console.log('Settings clicked') },
- ],
- }}
- SwitchProfileMenuProps={{
+ ]}
+ switchProfile={{
label: 'Switch Profile',
onClick: () => console.log('Switch Profile clicked'),
}}
- ProfilesListProps={{
- label: 'Profiles List',
- onClick: () => console.log('Profiles List clicked'),
- }}
- AddProfileMenuItemProps={{
- label: 'Add Profile',
- onClick: () => console.log('Add Profile clicked'),
- }}
- LogoutItemProps={{
- label: 'Logout',
- onClick: () => console.log('Logout clicked'),
+ profiles={{
+ list: [],
+ onAdd: () => console.log('Add Profile clicked'),
}}
+ onLogout={() => console.log('Logout clicked')}
/>
)
}📝 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.
| ```javascript | |
| import { AccountPopover } from '@baseapp-frontend/design-system' | |
| const MyComponent = () => { | |
| return ( | |
| <AccountPopover | |
| MenuItemsProps={{ | |
| menuItems: [ | |
| { label: 'Profile', onClick: () => console.log('Profile clicked') }, | |
| { label: 'Settings', onClick: () => console.log('Settings clicked') }, | |
| ], | |
| }} | |
| SwitchProfileMenuProps={{ | |
| label: 'Switch Profile', | |
| onClick: () => console.log('Switch Profile clicked'), | |
| }} | |
| ProfilesListProps={{ | |
| label: 'Profiles List', | |
| onClick: () => console.log('Profiles List clicked'), | |
| }} | |
| AddProfileMenuItemProps={{ | |
| label: 'Add Profile', | |
| onClick: () => console.log('Add Profile clicked'), | |
| }} | |
| LogoutItemProps={{ | |
| label: 'Logout', | |
| onClick: () => console.log('Logout clicked'), | |
| }} | |
| /> | |
| ) | |
| } | |
| export default MyComponent | |
| ``` | |
| import { AccountPopover } from '@baseapp-frontend/design-system' | |
| const MyComponent = () => { | |
| return ( | |
| <AccountPopover | |
| menuItems={[ | |
| { label: 'Profile', onClick: () => console.log('Profile clicked') }, | |
| { label: 'Settings', onClick: () => console.log('Settings clicked') }, | |
| ]} | |
| switchProfile={{ | |
| label: 'Switch Profile', | |
| onClick: () => console.log('Switch Profile clicked'), | |
| }} | |
| profiles={{ | |
| list: [], | |
| onAdd: () => console.log('Add Profile clicked'), | |
| }} | |
| onLogout={() => console.log('Logout clicked')} | |
| /> | |
| ) | |
| } | |
| export default MyComponent |
joaosoutto
force-pushed
the
BA-2016-missing-MDX-group2
branch
from
45e522d to
27ca7da
Compare
joaosoutto
force-pushed
the
BA-2016-missing-MDX-group2
branch
from
27ca7da to
f754a0b
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx (4)
9-10: Enhance accessibility documentationThe description is good, but could be more specific about accessibility features. Consider adding:
- ARIA roles and attributes used
- Keyboard navigation patterns
- Screen reader behavior
Also, fix these grammatical issues for better clarity:
-Opens as a modal overlay centered on the screen. Can be closed via a close button +The dialog opens as a modal overlay centered on the screen. It can be closed via a close button -pressing ESC key +pressing the ESC key🧰 Tools
🪛 LanguageTool
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...(MISSING_IT_THERE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...cking outside (if enabled), or pressing ESC key (if enabled). Should trap focus wit...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
23-25: Document additional common dialog propsConsider documenting these additional props for completeness:
maxWidth: Maximum width of the dialogfullWidth: If true, the dialog stretches to maximum widthfullScreen: If true, the dialog fills the screendisableEscapeKeyDown: Disable the ESC key handlerdisableBackdropClick: Disable clicking outside to close
29-32: Add cross-references to related componentsConsider adding links to the documentation of related components to help users navigate between them:
-- ConfirmDialog: A specialized version for confirmation actions +- [ConfirmDialog](/docs/designSystem-dialogs-confirmdialog--docs): A specialized version for confirmation actions
36-49: Enhance example usage sectionConsider adding more comprehensive examples:
- Dialog with custom width and fullScreen behavior
- Dialog with form elements showing focus management
- Dialog with custom close behavior
- Dialog with different content layouts
Example:
const MyComponent = () => { const [open, setOpen] = useState(false); const handleClose = (event, reason) => { if (reason !== 'backdropClick') { setOpen(false); } }; return ( <Dialog open={open} onClose={handleClose} maxWidth="md" fullWidth disableBackdropClick > <DialogTitle>Custom Dialog</DialogTitle> <DialogContent> <form> {/* Form elements */} </form> </DialogContent> <DialogActions> <Button onClick={() => setOpen(false)}>Close</Button> </DialogActions> </Dialog> ); };packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx (5)
1-3: Standardize the Storybook title format.Consider using forward slashes consistently throughout the title for better hierarchy:
-<Meta title="@baseapp-frontend | designSystem/Displays/LoadingState" /> +<Meta title="@baseapp-frontend/Design System/Displays/LoadingState" />
9-10: Add accessibility information to the component overview.Consider adding details about the component's accessibility features, such as ARIA attributes or screen reader behavior.
24-24: Enhance props documentation with more details and links.Consider:
- Adding a link to the MUI CircularProgress documentation
- Including common CircularProgressProps that users might want to customize
- Adding a props table with type information
28-31: Clarify relationships with related components.For each related component, explain when to use one over the other and provide specific scenarios. For example:
- When to use LoadingState vs. plain CircularProgress
- Specific examples of using LoadingState within Dialog or Popover
35-43: Expand example usage with more real-world scenarios.Consider adding:
- Multiple examples showing different use cases
- Error handling scenarios
- Integration with async operations
- Usage within containers (Dialog, Popover)
Example:
import { LoadingState } from '@baseapp-frontend/design-system' // Basic usage const SimpleExample = () => ( <LoadingState CircularProgressProps={{ size: 40 }} /> ) // Usage with async data fetching const DataFetchingExample = () => { const [isLoading, setIsLoading] = useState(true) return ( <div style={{ position: 'relative', minHeight: '200px' }}> {isLoading && <LoadingState />} {/* Content */} </div> ) } // Usage within Dialog const DialogExample = () => ( <Dialog open={true}> <LoadingState CircularProgressProps={{ size: 24 }} /> </Dialog> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/AccountPopover.mdx(1 hunks)packages/components/modules/navigations/Header/__storybook__/Header.mdx(1 hunks)packages/components/modules/navigations/NavCentered/__storybook__/NavCentered.mdx(1 hunks)packages/components/modules/navigations/NavHorizontal/__storybook__/NavHorizontal.mdx(1 hunks)packages/components/modules/navigations/NavMini/__storybook__/NavMini.mdx(1 hunks)packages/components/modules/navigations/NavVertical/__storybook__/NavVertical.mdx(1 hunks)packages/components/modules/navigations/NavigationLayout/__storybook__/NavigationLayout.mdx(1 hunks)packages/design-system/components/Scrollbar/__storybook__/Scrollbar.mdx(1 hunks)packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx(1 hunks)packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/design-system/components/Scrollbar/storybook/Scrollbar.mdx
- packages/components/modules/navigations/NavCentered/storybook/NavCentered.mdx
- packages/components/modules/navigations/Header/storybook/Header.mdx
- packages/components/modules/navigations/NavHorizontal/storybook/NavHorizontal.mdx
- packages/components/modules/navigations/Header/AccountMenu/AccountPopover/storybook/AccountPopover.mdx
- packages/components/modules/navigations/NavMini/storybook/NavMini.mdx
- packages/components/modules/navigations/NavigationLayout/storybook/NavigationLayout.mdx
- packages/components/modules/navigations/NavVertical/storybook/NavVertical.mdx
🧰 Additional context used
🪛 LanguageTool
packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx
[style] ~10-~10: To form a complete sentence, be sure to include a subject.
Context: ...a modal overlay centered on the screen. Can be closed via a close button, clicking ...
(MISSING_IT_THERE)
[uncategorized] ~10-~10: You might be missing the article “the” here.
Context: ...cking outside (if enabled), or pressing ESC key (if enabled). Should trap focus wit...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Applications and Packages
- GitHub Check: Lint
🔇 Additional comments (2)
packages/design-system/components/dialogs/Dialog/__storybook__/BaseDialog.mdx (1)
1-3: Verify title structure consistencyThe title structure looks good, but let's ensure it follows the same pattern as other MDX files in the codebase.
✅ Verification successful
Title structure is consistent with other MDX files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check title structure consistency across MDX files # Expected pattern: "@baseapp-frontend | designSystem/<category>/<component>" # Search for Meta title patterns in MDX files rg -t mdx '<Meta title="@baseapp-frontend \| designSystem/' -A 1Length of output: 94
Script:
#!/bin/bash # Find all MDX files and search for Meta title patterns fd '.mdx$' | xargs rg '<Meta title=".*designSystem/' # Alternative search to catch potential variations fd '.mdx$' | xargs rg 'title="@baseapp-frontend.*designSystem'Length of output: 1563
packages/design-system/components/displays/LoadingState/__storybook__/LoadingState.mdx (1)
14-14: Enhance the Current Usage section with specific examples.As per the previous review feedback, please provide more specific and direct information about how the component is currently being used within the package. Instead of a generic description, include concrete examples of where and how it's implemented.
Let's find specific usage examples in the codebase:
✅ Verification successful
Update Current Usage section with concrete examples
The Current Usage section should be updated to reflect the actual implementation patterns found in the codebase:
- **Current Usage**: Used as a loading indicator in several key scenarios: - Suspense boundaries for async component loading - Infinite scroll pagination in lists (messages, notifications, comments) - Initial data fetching states in profile and member views - Processing feedback during user interactions🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for actual usage of LoadingState component rg -l "LoadingState" --type ts --type tsx | grep -v "__storybook__" | grep -v "\.stories\."Length of output: 117
Script:
#!/bin/bash # Find all TypeScript and React files echo "=== TypeScript and React files containing LoadingState ===" fd "\.(ts|tsx)$" | xargs rg "LoadingState" -l | grep -v "__storybook__" | grep -v "\.stories\." echo -e "\n=== Usage context (showing 3 lines before and after) ===" fd "\.(ts|tsx)$" | xargs rg "LoadingState" -B 3 -A 3 | grep -v "__storybook__" | grep -v "\.stories\." echo -e "\n=== Import statements ===" fd "\.(ts|tsx)$" | xargs rg "import.*LoadingState" | grep -v "__storybook__" | grep -v "\.stories\."Length of output: 46510
| - **Current Usage**: Used in confirmation flows, detailed information display, form submissions, and important user notifications. | ||
| - **Potential Usage**: Could be used for: | ||
| - User preferences or settings panels | ||
| - Detailed item previews | ||
| - Multi-step forms | ||
| - Error or success messages requiring acknowledgment |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify current usage examples per reviewer's feedback
As requested by @deboracosilveira, the "Current Usage" section should be more direct and specific. Consider:
- Providing concrete examples from the actual codebase
- Including code snippets for each use case
- Removing generic descriptions in favor of specific implementations
Original Story:
Acceptance Criteria
Context:
The BaseApp Frontend Packages https://github.com/silverlogic/baseapp-frontend contains a number of pages and components that currently have Storybook stories but are missing corresponding MDX documentation files.
Create MDX Documentation Files: For each missing component or page listed below, create an .mdx file in the storybook folder within the component’s or page's directory.
Follow Documentation Standards: Use the MDX Documentation Template provided in our Tettra guide https://app.tettra.co/teams/TSL/pages/frontend-documentation-guide
Please check examples on:
https://bitbucket.org/silverlogic/baseapp-frontend-template/src/master/apps/web/components/design-system/inputs/PasswordField/__storybook__/PasswordField.mdx
https://bitbucket.org/silverlogic/baseapp-frontend-template/src/master/apps/web/app/(with-navigation)/__storybook__/HomePage.mdx
Missing MDX:
BaseDialog
Scrollbar
LoadingState
Header
AccountPopover
NavMini
NavigationLayout
NavCentered
NavVertical
NavHorizontal
Current behavior
Expected behavior
Code Snippet
Approvd
https://app.approvd.io/projects/BA/stories/36741
Summary by CodeRabbit
Documentation
New Features