Skip to content

Conversation

@junioresc
Copy link
Contributor

@junioresc junioresc commented Jan 21, 2025

Original Story:
Acceptance Criteria
Context:
The BaseApp Frontend Template https://bitbucket.org/silverlogic/baseapp-frontend-template/ project 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:

LazyLoadImage

Searchbar

PhoneNumberField

FormattableTextField

LinearLoadingScreen

LoadingScreen

FeedbackScreen

Current behavior

Expected behavior

Code Snippet

Approvd
https://app.approvd.io/silverlogic/BA/stories/36740

Summary by CodeRabbit

  • Documentation
    • Added comprehensive Storybook documentation for the Searchbar component.
    • Detailed component description, use cases, and prop specifications.
    • Included example usage demonstrating integration and state management.
    • Introduced new documentation for the ImageWithFallback component.
    • Provided detailed use cases and prop specifications for the ImageWithFallback component.
  • Chores
    • Updated version for @baseapp-frontend/design-system to 0.0.34.
    • Updated version for @baseapp-frontend/wagtail to 1.0.19.
    • Updated CHANGELOG entries for both design system and wagtail packages.

@changeset-bot
Copy link

changeset-bot bot commented Jan 21, 2025

⚠️ No Changeset found

Latest commit: a5a4cb7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2025

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/design-system/components/images/ImageWithFallback/__storybook__/stories.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /packages/design-system/.eslintrc.js
Error: Cannot find module '@baseapp-frontend/config/.eslintrc.js'
Require stack:

  • /packages/design-system/.eslintrc.js
  • /node_modules/.pnpm/@eslint+eslintrc@2.1.4/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/eslint/index.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/cli.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Module._load (node:internal/modules/cjs/loader:1074:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)
    at require (node:internal/modules/helpers:135:16)
    at Object. (/packages/design-system/.eslintrc.js:1:18)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)

Walkthrough

The pull request introduces comprehensive Storybook documentation for the Searchbar component, detailing its purpose, functionality, and usage, including prop descriptions, use cases, and an example implementation. Additionally, version updates and changelogs are included for related packages reflecting dependency changes, such as updates to the @baseapp-frontend/design-system and @baseapp-frontend/wagtail packages. New documentation for the ImageWithFallback component is also provided.

Changes

File Change Summary
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx Added Storybook documentation with Meta tag, component description, prop details, use cases, and example usage.
packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx Added new documentation for the ImageWithFallback component in Storybook.
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx Updated title for the Searchbar component and removed argTypes for isPending, onClear, and onChange.
packages/design-system/components/images/ImageWithFallback/__storybook__/stories.tsx Updated title for the ImageWithFallback component in Storybook.
packages/components/CHANGELOG.md Added new version 0.0.55, updated dependency @baseapp-frontend/design-system to 0.0.34.
packages/components/package.json Updated version from 0.0.54 to 0.0.55.
packages/design-system/CHANGELOG.md Added new version 0.0.34 reflecting documentation addition for the Searchbar and ImageWithFallback components.
packages/design-system/package.json Updated version from 0.0.33 to 0.0.34.
packages/wagtail/CHANGELOG.md Added new version 1.0.19 indicating dependency update for @baseapp-frontend/design-system.
packages/wagtail/package.json Updated version from 1.0.18 to 1.0.19.

Possibly related PRs

  • BA-1584 Add Storybook for Notifications #128: The main PR introduces documentation for the Searchbar component, while this PR adds Storybook for notifications components, which may include similar documentation practices and structure.
  • BA-2086-fe-storybook-design-system-dropzone #181: This PR focuses on enhancing the Dropzone component with Storybook documentation, similar to the documentation efforts in the main PR for the Searchbar.
  • BA-2116 System messages #186: This PR introduces a new SystemMessage component, which may have documentation needs similar to those addressed in the main PR for the Searchbar component.

Suggested reviewers

  • deboracosilveira
  • priscilladeroode
  • anicioalexandre

🐰 A Rabbit's Ode to Searchbar Docs
Searching high and searching low,
Documentation now begins to grow
Storybook pages crisp and bright
Guiding devs with pure delight!
🔍✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch from 4d32674 to b2d3259 Compare January 21, 2025 18:30
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (4)

1-3: Standardize the Meta title format.

The Meta title format should be adjusted for consistency:

  1. Use "/" instead of " / " as the separator
  2. Verify if the prefix should be "@baseapp-frontend" instead of "@baseapp-frontend-template" to match the package name
-<Meta title="@baseapp-frontend-template / inputs/Searchbar" />
+<Meta title="@baseapp-frontend/inputs/Searchbar" />

9-10: Add accessibility documentation.

Since this is a search input component, please document:

  1. ARIA attributes used (e.g., aria-label, role="search")
  2. Keyboard interactions (e.g., Esc to clear)
  3. Screen reader considerations

35-52: Enhance the example with TypeScript and loading state.

The example could be improved by:

  1. Adding TypeScript types
  2. Showing how to handle the loading state
  3. Including all necessary imports
+import React, { useState } from 'react'
 import Searchbar from '../Searchbar'
 
-const MyComponent = () => {
+const MyComponent: React.FC = () => {
   const [searchValue, setSearchValue] = useState('')
+  const [isLoading, setIsLoading] = useState(false)
+
+  const handleSearch = async (e: React.ChangeEvent<HTMLInputElement>) => {
+    const value = e.target.value
+    setSearchValue(value)
+    if (value) {
+      setIsLoading(true)
+      // Simulate API call
+      await new Promise(resolve => setTimeout(resolve, 1000))
+      setIsLoading(false)
+    }
+  }

   return (
     <Searchbar
       placeholder="Search"
       value={searchValue}
-      onChange={(e) => setSearchValue(e.target.value)}
+      onChange={handleSearch}
       onClear={() => setSearchValue('')}
-      isPending={false}
+      isPending={isLoading}
     />
   )
 }

5-8: Add essential documentation sections.

Consider adding these important sections to make the documentation more comprehensive:

  1. Component variants and customization examples
  2. Visual examples of do's and don'ts
  3. Troubleshooting guide
  4. Best practices
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d32674443a1a50544fbd121356cb1e3dcae4a5e and b2d32598f302d65a0980ad464aee24ccc9b6e29c.

📒 Files selected for processing (7)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/package.json (1 hunks)
  • packages/design-system/CHANGELOG.md (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1 hunks)
  • packages/design-system/package.json (1 hunks)
  • packages/wagtail/CHANGELOG.md (1 hunks)
  • packages/wagtail/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (6)
  • packages/components/package.json
  • packages/wagtail/package.json
  • packages/design-system/CHANGELOG.md
  • packages/design-system/package.json
  • packages/wagtail/CHANGELOG.md
  • packages/components/CHANGELOG.md
🔇 Additional comments (1)
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1)

1-52: Documentation meets basic requirements but could be enhanced.

The MDX documentation successfully provides the basic structure and information required. While it meets the PR objectives, consider implementing the suggested improvements in a follow-up PR to make the documentation more comprehensive and valuable for developers.

@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch from b2d3259 to 0195fcd Compare January 22, 2025 17:32
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)

Line range hint 16-20: Add more story variants to showcase component functionality.

The current stories only demonstrate the default state with isPending. Consider adding stories that showcase:

  • Search in progress (isPending: true)
  • With initial value
  • With error state
  • Search with results
  • Search with no results
  • Interaction with onClear
  • Disabled state

This will help document the component's various states and behaviors.

Example addition:

export const Default: Story = {
  args: {
    isPending: false,
  },
}

+export const Searching: Story = {
+  args: {
+    isPending: true,
+    value: 'search query',
+  },
+}
+
+export const WithError: Story = {
+  args: {
+    error: 'Invalid search query',
+    value: '!@#',
+  },
+}
🧹 Nitpick comments (1)
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)

Line range hint 1-20: Document prop types in the story configuration.

While the props are documented in the MDX file, it's beneficial to include argTypes in the story configuration to:

  1. Provide interactive controls in Storybook
  2. Show prop descriptions inline
  3. Define proper control types for better testing

Example addition:

const meta: Meta<WithControllerProps<SearchbarProps>> = {
  title: '@baseapp-frontend | designSystem/Inputs/Searchbar',
  component: Searchbar,
+  argTypes: {
+    isPending: {
+      control: 'boolean',
+      description: 'Indicates if a search is in progress',
+    },
+    value: {
+      control: 'text',
+      description: 'Current search query value',
+    },
+    onChange: {
+      description: 'Callback fired when the search query changes',
+    },
+    onClear: {
+      description: 'Callback fired when the clear button is clicked',
+    },
+    error: {
+      control: 'text',
+      description: 'Error message to display',
+    },
+  },
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2d32598f302d65a0980ad464aee24ccc9b6e29c and 0195fcdb0f46e389be52445fc7f81480f15c7196.

📒 Files selected for processing (8)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/package.json (1 hunks)
  • packages/design-system/CHANGELOG.md (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1 hunks)
  • packages/design-system/package.json (1 hunks)
  • packages/wagtail/CHANGELOG.md (1 hunks)
  • packages/wagtail/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/design-system/CHANGELOG.md
  • packages/wagtail/package.json
  • packages/components/package.json
  • packages/wagtail/CHANGELOG.md
  • packages/components/CHANGELOG.md
  • packages/design-system/package.json
  • packages/design-system/components/inputs/Searchbar/storybook/Searchbar.mdx
🔇 Additional comments (1)
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)

9-9: LGTM! Improved component organization.

The updated title path better reflects the component's category in the design system hierarchy.

@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch from 0195fcd to e23e524 Compare January 22, 2025 17:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)

Line range hint 16-20: Remove unnecessary args object.

Since we're removing argTypes as per previous feedback, we should also remove the args object from the Default story as it's not needed anymore. This will keep the stories file minimal and consistent with other components.

export const Default: Story = {
-  args: {
-    isPending: false,
-  },
}
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (3)

37-54: Add missing imports to the example code.

The example code should include all necessary imports for completeness:

+import { useState } from 'react'
 import Searchbar from '../Searchbar'

Also, consider adding a comment explaining the state management pattern shown in the example.


30-33: Add import paths for related components.

Include the import paths for the related components to make it easier for developers to find and use them:

- PureTextField: The base text field component used internally.
- IconButton: Used for the clear button.
- CircularProgress: Used for the loading indicator.
+ PureTextField: The base text field component used internally. (@baseapp-frontend/design-system/components/inputs/PureTextField)
+ IconButton: Used for the clear button. (@mui/material/IconButton)
+ CircularProgress: Used for the loading indicator. (@mui/material/CircularProgress)

14-15: Expand current usage examples.

The current usage section could be more specific about where the Searchbar is actually being used in the application. Consider adding concrete examples of existing implementations to help developers understand real-world use cases.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0195fcdb0f46e389be52445fc7f81480f15c7196 and e23e524b12565e5e6b5febd06e5b778f0c8a7973.

📒 Files selected for processing (8)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/package.json (1 hunks)
  • packages/design-system/CHANGELOG.md (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1 hunks)
  • packages/design-system/package.json (1 hunks)
  • packages/wagtail/CHANGELOG.md (1 hunks)
  • packages/wagtail/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/design-system/CHANGELOG.md
  • packages/wagtail/package.json
  • packages/components/package.json
  • packages/wagtail/CHANGELOG.md
  • packages/design-system/package.json
  • packages/components/CHANGELOG.md
🔇 Additional comments (1)
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1)

19-26: Enhance props documentation.

The props documentation needs the following improvements:

  1. Add type information for sx prop (e.g., SxProps<Theme>)
  2. Add type information for InputProps (e.g., TextFieldProps['InputProps'])
  3. Add allowed values for variant prop (e.g., 'filled' | 'outlined' | 'standard')

@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch 2 times, most recently from c8f9e91 to c92ea43 Compare January 22, 2025 17:51
@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch from c92ea43 to 4f2217d Compare January 22, 2025 19:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1)

14-15: Enhance use cases with specific examples.

While the current and potential usage sections provide good general context, consider adding specific examples of where this component is currently used in the application to help other developers understand its practical applications.

Example additions:

 - **Current Usage**: Used in search interfaces within the application.
+    - Example: Used in the main navigation bar for global search
+    - Example: Used in the user management page for filtering users
packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx (5)

10-10: Fix grammatical error in the expected behavior description.

The sentence structure needs improvement for better clarity.

- - **Expected Behavior**: The component renders an image field. It uses `src` by default but the fallback `fallbackSrc` case src fails to load.
+ - **Expected Behavior**: The component renders an image field. It uses `src` by default but uses the fallback `fallbackSrc` in case src fails to load.

15-15: Fix grammatical error in the potential usage description.

There's a subject-verb agreement error in the sentence.

- - **Potential Usage**: The `ImageWithFallback` could be used for fallback images to support multiple browsers like loading webp for modern browsers and png for browsers that doesn't support it.
+ - **Potential Usage**: The `ImageWithFallback` could be used for fallback images to support multiple browsers like loading webp for modern browsers and png for browsers that don't support it.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~15-~15: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...dern browsers and png for browsers that doesn't support it. ## Props - src (st...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)


19-25: Enhance props documentation with additional details.

Consider adding more information about:

  • Which props are required vs optional
  • Validation constraints for width and height (e.g., minimum values)
  • Supported image types for type and fallbackType
- - **src** (string): source of the image.
- - **fallbackSrc** (string): fallback source image.
- - **type** (string, optional): type of the src image (default 'image/webp')
- - **fallbackType** (string, optional): type of the fallback image (default 'image/png')
- - **alt** (string): An alternate text for the image
- - **width** (number): Width of the image
- - **height** (number): Height of the image
+ - **src** (string, required): Source URL of the primary image.
+ - **fallbackSrc** (string, required): Source URL of the fallback image to use when the primary image fails to load.
+ - **type** (string, optional): MIME type of the primary image (default: 'image/webp', supported: 'image/webp', 'image/png', 'image/jpeg')
+ - **fallbackType** (string, optional): MIME type of the fallback image (default: 'image/png', supported: 'image/webp', 'image/png', 'image/jpeg')
+ - **alt** (string, required): Alternative text for accessibility and SEO
+ - **width** (number, required): Width of the image in pixels (must be > 0)
+ - **height** (number, required): Height of the image in pixels (must be > 0)

29-30: Enhance the related components section with more context.

The relationship between ImageWithFallback and Image components needs more explanation.

- - **Related Components**:
-   - `Image`: Used to render the image in case src fails to load.
+ - **Related Components**:
+   - `Image`: The base component used internally by ImageWithFallback to handle image rendering. ImageWithFallback extends its functionality by adding fallback source handling when the primary image fails to load.

34-46: Enhance the example usage section.

Consider improving the example with:

  • A more descriptive import statement
  • Comments explaining the props
  • Multiple examples showing different use cases (e.g., with and without optional props)
 ```javascript
-import ImageWithFallback from '../ImageWithFallback'
+// Import from the design system package
+import { ImageWithFallback } from '@baseapp-frontend/design-system'
 
-const MyComponent = () => (
+// Basic usage with required props
+const BasicExample = () => (
   <ImageWithFallback
     src="/webp/home-banner.webp"
     fallbackSrc="/webp/home-banner.png"
     alt="Home Banner"
     width={300}
     height={400}
   />
 )
-export default MyComponent
+
+// Advanced usage with optional props
+const AdvancedExample = () => (
+  <ImageWithFallback
+    src="/images/profile.webp"
+    fallbackSrc="/images/profile.jpg"
+    type="image/webp"
+    fallbackType="image/jpeg"
+    alt="User Profile"
+    width={200}
+    height={200}
+  />
+)
+
+export { BasicExample, AdvancedExample }

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between e23e524b12565e5e6b5febd06e5b778f0c8a7973 and 4f2217d52a6a3463ae166703db58082bb1ea0252.

</details>

<details>
<summary>📒 Files selected for processing (10)</summary>

* `packages/components/CHANGELOG.md` (1 hunks)
* `packages/components/package.json` (1 hunks)
* `packages/design-system/CHANGELOG.md` (1 hunks)
* `packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx` (1 hunks)
* `packages/design-system/components/images/ImageWithFallback/__storybook__/stories.tsx` (1 hunks)
* `packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx` (1 hunks)
* `packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx` (1 hunks)
* `packages/design-system/package.json` (1 hunks)
* `packages/wagtail/CHANGELOG.md` (1 hunks)
* `packages/wagtail/package.json` (1 hunks)

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>

* packages/design-system/components/images/ImageWithFallback/__storybook__/stories.tsx

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (6)</summary>

* packages/design-system/CHANGELOG.md
* packages/design-system/package.json
* packages/components/package.json
* packages/wagtail/package.json
* packages/wagtail/CHANGELOG.md
* packages/components/CHANGELOG.md

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx</summary>

[uncategorized] ~15-~15: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...dern browsers and png for browsers that doesn't support it.  ## Props  - **src** (st...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (1)</summary>

* GitHub Check: Analyze (javascript)

</details>

<details>
<summary>🔇 Additional comments (5)</summary><blockquote>

<details>
<summary>packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)</summary>

`9-9`: **LGTM! Title updated as requested.**

The component title has been correctly updated to include the 'Inputs' path, and unnecessary configuration (tags, argTypes) has been removed as requested.

</details>
<details>
<summary>packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (3)</summary>

`1-3`: **LGTM! Meta title is consistent.**

The Meta title matches the stories configuration, following the requested path structure.

---

`19-26`: **Props documentation follows best practices.**

The props documentation is well-structured with:
- Clear indication of required vs optional props
- Detailed type information for callbacks
- All props shown in the example are documented

---

`1-54`: **Documentation is comprehensive and well-structured.**

The MDX documentation successfully:
- Provides clear purpose and behavior descriptions
- Includes comprehensive props documentation
- Offers practical example usage
- Follows the requested documentation standards

This meets the PR objective of adding missing MDX documentation for the Searchbar component.

</details>
<details>
<summary>packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx (1)</summary>

`1-3`: **LGTM! Meta configuration is properly structured.**

The title follows the correct naming convention for the design system components.

</details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@junioresc junioresc force-pushed the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch from 4f2217d to a5a4cb7 Compare January 23, 2025 17:57
@sonarqubecloud
Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx (3)

9-10: Fix grammar and clarity in Expected Behavior section.

The description contains grammatical errors and unclear wording.

Apply this diff to improve clarity:

-- **Expected Behavior**: The component renders an image field. It uses `src` by default but the fallback `fallbackSrc` case src fails to load.
+- **Expected Behavior**: The component renders an image field. It uses `src` by default, but falls back to `fallbackSrc` in case the primary source fails to load.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~10-~10: Possible missing comma found.
Context: ...an image field. It uses src by default but the fallback fallbackSrc case src fai...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~10-~10: Possible missing preposition found.
Context: ... default but the fallback fallbackSrc case src fails to load. ## Use Cases - **C...

(AI_HYDRA_LEO_MISSING_IN)


17-26: Enhance props documentation.

The props section needs improvements in type information and required/optional indicators.

Apply these changes to improve the props documentation:

 ## Props

-- **src** (string): source of the image.
-- **fallbackSrc** (string): fallback source image.
-- **type** (string, optional): type of the src image (default 'image/webp')
-- **fallbackType** (string, optional): type of the fallback image (default 'image/png')
-- **alt** (string): An alternate text for the image
-- **width** (number): Width of the image
-- **height** (number): Height of the image
+- **src** (string, required): Source URL of the primary image.
+- **fallbackSrc** (string, required): Source URL of the fallback image.
+- **type** (string, optional): MIME type of the primary image (default: 'image/webp')
+- **fallbackType** (string, optional): MIME type of the fallback image (default: 'image/png')
+- **alt** (string, required): Alternative text for accessibility
+- **width** (number, required): Width of the image in pixels
+- **height** (number, required): Height of the image in pixels

32-47: Enhance example with TypeScript and proper imports.

The example could be improved with TypeScript types and a proper import path.

Consider updating the example:

-import ImageWithFallback from '../ImageWithFallback'
+import { ImageWithFallback } from '@baseapp-frontend/design-system'

-const MyComponent = () => (
+const MyComponent: React.FC = () => (
   <ImageWithFallback
     src="/webp/home-banner.webp"
     fallbackSrc="/webp/home-banner.png"
     alt="Home Banner"
     width={300}
     height={400}
   />
 )
packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (2)

17-27: Add default values and type imports to props documentation.

While the props documentation is much improved, it could be even better with default values and type information.

Consider adding:

  1. Default value for variant prop
  2. Type definition for sx prop (MUI's SxProps)
  3. Type definition for InputProps

Example:

-- **sx** (object, optional): MUI system props for custom styling.
-- **InputProps** (object, optional): Additional props passed to the underlying `PureTextField` component.
-- **variant** (string, optional): The variant of the text field. Defaults to 'filled'.
+- **sx** (SxProps, optional): MUI system props for custom styling.
+- **InputProps** (TextFieldProps['InputProps'], optional): Additional props passed to the underlying `PureTextField` component.
+- **variant** ('filled' | 'outlined' | 'standard', optional): The variant of the text field. Default: 'filled'

35-54: Enhance example with TypeScript and proper imports.

The example could be improved with TypeScript types and better practices.

Consider updating the example:

-import Searchbar from '../Searchbar'
+import { useState } from 'react'
+import { Searchbar } from '@baseapp-frontend/design-system'

-const MyComponent = () => {
+const MyComponent: React.FC = () => {
-  const [searchValue, setSearchValue] = useState('')
+  const [searchValue, setSearchValue] = useState<string>('')

+  const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+    setSearchValue(e.target.value)
+  }
+
+  const handleClear = () => {
+    setSearchValue('')
+  }

   return (
     <Searchbar
       placeholder="Search"
       value={searchValue}
-      onChange={(e) => setSearchValue(e.target.value)}
-      onClear={() => setSearchValue('')}
+      onChange={handleChange}
+      onClear={handleClear}
       isPending={false}
     />
   )
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f2217d52a6a3463ae166703db58082bb1ea0252 and a5a4cb7.

📒 Files selected for processing (10)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/package.json (1 hunks)
  • packages/design-system/CHANGELOG.md (1 hunks)
  • packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx (1 hunks)
  • packages/design-system/components/images/ImageWithFallback/__storybook__/stories.tsx (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1 hunks)
  • packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1 hunks)
  • packages/design-system/package.json (1 hunks)
  • packages/wagtail/CHANGELOG.md (1 hunks)
  • packages/wagtail/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
  • packages/design-system/CHANGELOG.md
  • packages/components/package.json
  • packages/design-system/components/images/ImageWithFallback/storybook/stories.tsx
  • packages/wagtail/package.json
  • packages/design-system/package.json
  • packages/components/CHANGELOG.md
  • packages/wagtail/CHANGELOG.md
  • packages/design-system/components/inputs/Searchbar/storybook/stories.tsx
🧰 Additional context used
🪛 LanguageTool
packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx

[uncategorized] ~10-~10: Possible missing comma found.
Context: ...an image field. It uses src by default but the fallback fallbackSrc case src fai...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~10-~10: Possible missing preposition found.
Context: ... default but the fallback fallbackSrc case src fails to load. ## Use Cases - **C...

(AI_HYDRA_LEO_MISSING_IN)

🔇 Additional comments (2)
packages/design-system/components/images/ImageWithFallback/__storybook__/ImageWithFallback.mdx (1)

1-3: LGTM! Title follows the correct naming convention.

The Meta title follows the established pattern: "@baseapp-frontend | designSystem/[Category]/[Component]".

packages/design-system/components/inputs/Searchbar/__storybook__/Searchbar.mdx (1)

1-3: LGTM! Title has been updated as requested.

The Meta title now correctly follows the pattern "@baseapp-frontend | designSystem/Inputs/Searchbar" as requested in previous reviews.

@junioresc junioresc merged commit e2e807c into master Jan 23, 2025
9 checks passed
@junioresc junioresc deleted the BA-2141-add-missing-mdx-baseapp-frontend-template-group-3 branch January 23, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants