Fix typescript compilation errors on all presentational components #385
Conversation
WalkthroughThe pull request introduces a comprehensive refactoring of Storybook stories and component files across multiple presentational components in the project. The primary changes involve updating the Storybook story definitions to use the newer Each component's story file has been restructured to use a more explicit and type-safe approach to defining stories. This includes creating a Additionally, many component files have been updated to name their default exported functions, which improves debugging and stack trace clarity. These changes do not modify the fundamental functionality of the components but enhance code readability and maintainability. The refactoring appears to be part of a systematic update to align the project's Storybook configuration with more modern TypeScript and Storybook practices. 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: 25
🔭 Outside diff range comments (5)
src/components/presentational/WeekCalendar/WeekCalendar.tsx (1)
Line range hint
53-60: Remove redundant scroll position setting.The scroll position is being set twice - once in useEffect and once directly in the component body. This could cause unnecessary re-renders and potential flickering.
- if (element.current) { - element.current.scrollLeft = window.innerWidth; - } useEffect(() => { if (element.current) { element.current.scrollLeft = window.innerWidth; } - }); + }, []); // Add dependency array to prevent unnecessary re-runssrc/components/presentational/SingleExternalAccount/SingleExternalAccount.tsx (3)
Line range hint
31-33: Enhance error handling in refresh function.The promise returned by
MyDataHelps.refreshExternalAccountis not properly handled. Consider handling potential errors and updating the status accordingly.function refresh() { setStatusOverride("fetchingData"); - MyDataHelps.refreshExternalAccount(props.externalAccount.id).then(); + MyDataHelps.refreshExternalAccount(props.externalAccount.id) + .catch((error) => { + setStatusOverride("error"); + console.error('Failed to refresh external account:', error); + }); }
Line range hint
82-86: Replace javascript:{} href with button elements.Using
href="javascript:{}"is an anti-pattern. Consider using button elements instead, which are more semantic and accessible.-<a href="javascript:{}" onClick={refresh}> +<UnstyledButton onClick={refresh}> <FontAwesomeSvgIcon icon={faRepeat} /> {language("external-account-refresh")} -</a> +</UnstyledButton> -<a href="javascript:{}" onClick={() => props.onReconnectAccount(props.externalAccount)}> +<UnstyledButton onClick={() => props.onReconnectAccount(props.externalAccount)}> <FontAwesomeSvgIcon icon={faRepeat} /> {language("external-account-reconnect")} -</a> +</UnstyledButton>Also applies to: 89-93
Line range hint
21-29: Add confirmation step for account deletion.Consider adding a confirmation dialog before deleting an account to prevent accidental deletions.
function removeAccount() { + if (!window.confirm(language("external-account-remove-confirmation"))) { + return; + } setStatusOverride("deleting"); MyDataHelps.deleteExternalAccount(props.externalAccount.id).then(function () { props.onAccountRemoved(props.externalAccount); }).catch(function () { setStatusOverride(""); }); }src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.tsx (1)
Line range hint
84-84: Fix type annotation in map functionReplace the
anytype with the properSparklinePointinterface:-props.labResultValue.SparklinePoints.slice(0, props.labResultValue.SparklinePoints.length - 1).map((point: any, index: number) => +props.labResultValue.SparklinePoints.slice(0, props.labResultValue.SparklinePoints.length - 1).map((point: SparklinePoint, index: number) =>
🧹 Nitpick comments (35)
src/components/presentational/Layout/Layout.tsx (1)
31-31: LGTM! Good improvement to component naming.The change from an anonymous function to a named export improves debugging, stack traces, and component identification. This aligns with React best practices.
Consider adding a JSDoc comment above the component to document its purpose and usage:
+/** + * Layout component that provides theming, styling and safe area handling. + * Wraps content with necessary context providers and global styles. + */ export default function Layout(props: LayoutProps) {src/components/presentational/SingleNotification/SingleNotification.stories.tsx (2)
19-23: Minor improvements for render functionThe render function has inconsistent indentation. Consider letting TypeScript infer the args type.
-const render = (args: SingleNotificationProps) => <Layout> +const render = (args) => <Layout> <Card> <SingleNotification {...args} /> </Card> </Layout>;
25-41: Consider extracting test dataThe notification object could be moved to a separate mock data file for better maintainability and reuse across stories.
Consider creating a
mockData.tsfile:// mockData.ts export const mockNotification = { id: "4", identifier: "SurveyReminder", sentDate: addDays(new Date(), -1).toISOString(), statusCode: "Succeeded", type: "Push", content: { title: "Have you received your Fitbit? Connect it!" }, recipients: [], contentVersion: 1 };Then import and use it in the story:
+import { mockNotification } from './mockData'; export const Default: Story = { args: { - notification: { - "id": "4", - "identifier": "SurveyReminder", - "sentDate": addDays(new Date(), -1).toISOString(), - "statusCode": "Succeeded", - "type": "Push", - "content": { - "title": "Have you received your Fitbit? Connect it!" - }, - recipients: [], - contentVersion: 1 - } + notification: mockNotification }, render: render };src/components/presentational/WeekCalendar/WeekCalendar.tsx (2)
Line range hint
23-38: Improve scroll listener cleanup.The scroll listener cleanup might not handle all cases properly. Consider moving the listener removal logic to a separate cleanup function.
useEffect(() => { if (props.onStartDateChange) { var scrollListener = debounce(function (ev: Event) { if (element.current?.scrollLeft == 0) { props.onStartDateChange!(add(props.startDate, { weeks: -1 })); - element.current?.removeEventListener("scroll", scrollListener); } else if (element.current?.clientWidth && element.current?.scrollLeft == element.current.clientWidth * 2) { props.onStartDateChange!(add(props.startDate, { weeks: 1 })); - element.current?.removeEventListener("scroll", scrollListener); } }, 500); element.current?.addEventListener("scroll", scrollListener); return () => { + scrollListener.cancel(); // Cancel any pending debounced calls element.current?.removeEventListener("scroll", scrollListener); } } }, [props.startDate])
Line range hint
75-77: Use proper date comparison instead of string manipulation.The date comparison using string manipulation could lead to timezone-related issues. Consider using proper date comparison methods.
- if (props.selectedDate && formatISO(date).substr(0, 10) == formatISO(props.selectedDate).substr(0, 10)) { + if (props.selectedDate && + date.getFullYear() === props.selectedDate.getFullYear() && + date.getMonth() === props.selectedDate.getMonth() && + date.getDate() === props.selectedDate.getDate()) {src/components/presentational/SingleExternalAccount/SingleExternalAccount.tsx (2)
Line range hint
41-45: Enhance accessibility for status changes.Consider adding ARIA attributes to improve accessibility when status changes occur.
-<div ref={props.innerRef} className="mdhui-single-external-account"> +<div + ref={props.innerRef} + className="mdhui-single-external-account" + role="region" + aria-live="polite" + aria-atomic="true" +>
Line range hint
35-39: Optimize status-based rendering.Consider memoizing the status and restructuring the status-based rendering to reduce complexity and improve performance.
+ const currentStatus = React.useMemo(() => getStatus(), [statusOverride, props.externalAccount.status]); + const renderStatusContent = () => { + const statusMap = { + unauthorized: ( + <> + <p><span className="error">{language("external-account-authorization-expired")}</span></p> + <p> + <UnstyledButton onClick={() => props.onReconnectAccount(props.externalAccount)}> + <FontAwesomeSvgIcon icon={faRepeat} /> {language("external-account-reconnect")} + </UnstyledButton> + </p> + </> + ), + // Add other status renders... + }; + return statusMap[currentStatus] || null; + };Also applies to: 46-96
src/components/presentational/DumbbellChart/DumbbellChart.tsx (4)
5-5: Consider using tuple type for fixed-length arrayThe
valuesproperty inClosedIntervalrepresents a range with start and end values. Consider using a tuple type for better type safety.-export interface ClosedInterval { values: number[] }; +export interface ClosedInterval { values: [number, number] };
44-55: Extract style calculation logicThe style calculation logic could be extracted into a separate function for better readability and maintainability.
+function calculateAxisStyle(bottom: number, range: number): React.CSSProperties { + return { bottom: `${(bottom / range) * 100}%` }; +} -var style = { "bottom": `${0}%` }; +var style = calculateAxisStyle(0, _range); // ... -style = { "bottom": `${(bottom / _range) * 100}%` }; +style = calculateAxisStyle(bottom, _range);
48-51: Extract CSS class names as constantsConsider extracting hardcoded CSS class names into constants for better maintainability.
+const CSS_CLASSES = { + AXIS_TEXT: 'mdhui-dumbbell-axis-text', + AXIS_TEXT_HIDDEN: 'mdhui-dumbbell-axis-text-hidden' +} as const; -let axisTextClass: string[] = ["mdhui-dumbbell-axis-text"]; +let axisTextClass: string[] = [CSS_CLASSES.AXIS_TEXT]; if (i == increments || i == 0) { - axisTextClass.push("mdhui-dumbbell-axis-text-hidden"); + axisTextClass.push(CSS_CLASSES.AXIS_TEXT_HIDDEN); }
61-62: Consider memoizing dumbbells renderingThe dumbbells mapping could benefit from memoization to prevent unnecessary re-renders.
+const memoizedDumbbells = React.useMemo(() => + props.dumbbells.map((db, index) => ( + <Dumbbell + key={`mdhui-dumbbell-${index}`} + dumbbell={db} + axis={props.axis} + index={index + 1} + /> + )), + [props.dumbbells, props.axis] +); function buildDumbbells() { - return (props.dumbbells.map((db, index) => <Dumbbell key={`mdhui-dumbbell-${index}`} dumbbell={db} axis={props.axis} index={index + 1} />)); + return memoizedDumbbells; }src/components/presentational/Button/Button.tsx (1)
Line range hint
8-19: Consider type and accessibility improvements.A few suggestions to enhance the component:
- The
onClicktype could be more specific:-onClick: Function; +onClick: (event: React.MouseEvent<HTMLButtonElement>) => void;
- Add aria attributes for loading state:
<button ref={props.innerRef} + aria-busy={props.loading} + aria-live="polite" style={{ backgroundColor: props.disabled ? undefined : backgroundColor, color: props.disabled ? undefined : textColor }}src/components/presentational/Switch/Switch.tsx (1)
Line range hint
5-11: Improve type definitionsA few type-related improvements:
- Use lowercase
booleaninstead ofBooleanfor primitive types- Add specific type for
onBackgroundColorexport interface SwitchProps { - isOn: Boolean; + isOn: boolean; - onBackgroundColor?: string; + onBackgroundColor?: `#${string}` | `rgb(${number},${number},${number})` | `rgba(${number},${number},${number},${number})`; onValueChanged(value: boolean): void; className?: string; innerRef?: React.Ref<HTMLButtonElement>; }🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead(lint/complexity/noBannedTypes)
src/components/presentational/CardTitle/CardTitle.stories.tsx (1)
29-29: Document the empty callbackThe empty callback should be documented to indicate it's intentionally empty for the story.
- onDetailClick: () => { } + onDetailClick: () => { /* no-op for story display */ }src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.stories.tsx (2)
4-4: Avoid wildcard imports from @storybook/reactReplace the wildcard import with specific imports for better maintainability and tree-shaking.
-import { Meta, StoryObj } from "@storybook/react/*"; +import { Meta, StoryObj } from "@storybook/react";
23-36: Consider using a type-safe color systemThe hardcoded color values could benefit from a type-safe color system or constants to prevent errors and maintain consistency.
Consider creating a color constants file or using a design system:
// colors.ts export const Colors = { primary: { red: "#c4291c", orange: "#e35c33", // ... other colors }, secondary: { yellow: "#eec04c", // ... other colors } } as const;src/components/presentational/Calendar/Calendar.stories.tsx (3)
4-4: Avoid wildcard imports from @storybook/reactReplace the wildcard import with specific imports for better maintainability and tree-shaking.
-import { Meta, StoryObj } from "@storybook/react/*"; +import { Meta, StoryObj } from "@storybook/react";
Line range hint
19-28: Add return type to plainCalendarDay functionThe helper function should have an explicit return type for better type safety.
-let plainCalendarDay = function (year: number, month: number, day?: number) { +let plainCalendarDay = function (year: number, month: number, day?: number): JSX.Element {
31-32: Consider using static dates for consistent snapshotsUsing
new Date()in stories can cause snapshot tests to fail as the values change. Consider using a fixed date for consistency.- month: new Date().getMonth(), - year: new Date().getFullYear(), + month: 0, // January + year: 2024,src/components/presentational/SparkBarChart/SparkBarChart.tsx (1)
Line range hint
17-31: Consider memoizing the component for better performanceSince the component renders multiple bars and uses context, it could benefit from memoization to prevent unnecessary re-renders.
-export default function SparkBarChart(props: SparkBarChartProps) { +const SparkBarChart = React.memo(function SparkBarChart(props: SparkBarChartProps) { var width = (100 / props.bars.length); let context = useContext(LayoutContext); // ... rest of the implementation -} +}); +export default SparkBarChart;src/components/presentational/SparkBarChart/SparkBarChart.stories.tsx (2)
4-4: Avoid wildcard imports from @storybook/reactReplace the wildcard import with specific imports for better maintainability and tree-shaking.
-import { Meta, StoryObj } from "@storybook/react/*"; +import { Meta, StoryObj } from "@storybook/react";
17-21: Consider moving inline styles to CSSThe container's width styling could be moved to a CSS class for better maintainability.
+// Add to SparkBarChart.css +.spark-bar-chart-container { + width: 300px; +} -const render = (args: SparkBarChartProps) => <Layout colorScheme="auto"> - <div style={{ width: "300px" }}> +const render = (args: SparkBarChartProps) => <Layout colorScheme="auto"> + <div className="spark-bar-chart-container">src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx (1)
17-20: Consider memoizing the render functionThe render function is recreated on each render. Consider memoizing it with useCallback if it's used in a React context.
+import { useCallback } from 'react'; -const render = (args: SegmentedControlProps) => +const render = useCallback((args: SegmentedControlProps) => (<Layout colorScheme="auto"> <SegmentedControl {...args} /> -</Layout>) +</Layout>), []);src/components/presentational/Histogram/Histogram.tsx (1)
Line range hint
24-24: Simplify sorting logic for better readabilityThe current sorting logic combines value comparison and label comparison in a complex ternary expression.
-var sortedEntries = [...props.entries].sort((a, b) => b.value - a.value || ((a.label > b.label) ? 1 : ((b.label > a.label) ? -1 : 0))); +var sortedEntries = [...props.entries].sort((a, b) => { + const valueComparison = b.value - a.value; + if (valueComparison !== 0) return valueComparison; + return a.label.localeCompare(b.label); +});src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.tsx (2)
Line range hint
14-23: Improve gradient calculation code qualitySeveral improvements can be made to the gradient calculation logic:
- Use template literals instead of string concatenation
- Use strict equality comparison
- Use const/let instead of var
- var chunkPercent = 100 / props.primaryColors.length; - var background = "conic-gradient("; - var currentPercent = 0; - for (var i = 0; i < props.primaryColors.length; i++) { - background = background + props.primaryColors[i] + " " + currentPercent + "%, "; + const chunkPercent = 100 / props.primaryColors.length; + let background = "conic-gradient("; + let currentPercent = 0; + for (let i = 0; i < props.primaryColors.length; i++) { + background += `${props.primaryColors[i]} ${currentPercent}%, `; currentPercent += chunkPercent; - background = background + props.primaryColors[i] + " " + currentPercent + "%"; - if (i != props.primaryColors.length - 1) { + background += `${props.primaryColors[i]} ${currentPercent}%`; + if (i !== props.primaryColors.length - 1) { background += ", "; } }
Line range hint
25-25: Use const for immutable style objectThe style object is not reassigned, so it should use const instead of var.
- var style = { background: background }; + const style = { background };src/components/presentational/Face/Face.tsx (1)
19-19: Remove extra space after function nameFor consistency with other components like TrackerItem, remove the space between
functionand the opening parenthesis.-export default function Face (props: FaceProps) { +export default function Face(props: FaceProps) {src/components/presentational/Histogram/Histogram.stories.tsx (1)
4-4: Fix import path syntaxRemove the trailing "/*" from the @storybook/react import path.
-import { Meta, StoryObj } from "@storybook/react/*"; +import { Meta, StoryObj } from "@storybook/react";src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx (1)
21-21: Use const instead of varUsing
varis discouraged in modern JavaScript/TypeScript. Useconstfor values that won't be reassigned.-var currentDate = new Date(); +const currentDate = new Date();src/components/presentational/Action/Action.stories.tsx (1)
30-30: Consider using console.log instead of alert in storiesUsing
alert()in stories can block the UI and make testing more difficult. Consider usingconsole.log()for demonstration purposes.-onClick: () => alert("Clicked") +onClick: () => console.log("Clicked")Also applies to: 41-41, 51-51, 61-61
src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx (1)
30-30: Consider environment-specific URL configurationThe hard-coded API URL might not work across different environments.
Consider using an environment variable or configuration value for the API URL:
const API_BASE_URL = process.env.API_BASE_URL || 'https://mdhorg.ce.dev'; // Then use: `${API_BASE_URL}/api/v1/delegated/externalaccountproviders/37/logo`src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.stories.tsx (1)
27-29: Consider adding error handling for termInfo logging.The console.log in onViewTermInfo might be better suited for development purposes.
Consider this improvement:
- onViewTermInfo(termInfo : TermInformationReference) { - console.log(termInfo); - }, + onViewTermInfo(termInfo : TermInformationReference) { + if (process.env.NODE_ENV !== 'production') { + console.log('Term Info:', termInfo); + } + },src/components/presentational/DateRangeNavigator/DateRangeNavigator.tsx (1)
21-24: Consider simplifying the duration assignment.The nested ternary operators could be made more readable.
Consider this more maintainable approach:
- const duration: Duration = props.intervalType === "Month" ? { months: 1 } - : props.intervalType === "Day" ? { days: 1 } - : props.intervalType === "6Month" ? { months: 6 } - : { weeks: 1 }; + const duration: Duration = { + "Month": { months: 1 }, + "Day": { days: 1 }, + "6Month": { months: 6 }, + "Week": { weeks: 1 } + }[props.intervalType] || { weeks: 1 };src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx (1)
20-25: Consider extracting date initialization logicThe date initialization logic could be moved to a separate utility function for better reusability and testing.
+const getInitialIntervalStart = (currentDate: Date): Date => { + let intervalStart = new Date(currentDate); + while (intervalStart.getDay() !== 0) { + intervalStart = add(intervalStart, { days: -1 }); + } + return intervalStart; +}; let currentDate = new Date(); currentDate = new Date(currentDate.getFullYear(), currentDate.getMonth(), currentDate.getDate(), 0, 0, 0, 0); -var initialIntervalStart = currentDate; -while (initialIntervalStart.getDay() != 0) { - initialIntervalStart = add(initialIntervalStart, { days: -1 }); -} +const initialIntervalStart = getInitialIntervalStart(currentDate);src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.tsx (1)
Line range hint
8-21: Enhance type safety for interfacesConsider making the interfaces more type-safe:
export interface SparklinePoint { - X: number, - Y: number + X: number, // Normalized value between 0 and 1 + Y: number // Normalized value between 0 and 1 } export interface LabResultValue { Type: string, MostRecentValue: string, AcuityHighlight: string, MostRecentDate: string, - NormalRangeTopY: number, - NormalRangeBottomY: number, + NormalRangeTopY: number, // Normalized value between 0 and 1 + NormalRangeBottomY: number, // Normalized value between 0 and 1 SparklinePoints: SparklinePoint[], TermInformation?: TermInformationReference }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
src/components/presentational/Action/Action.stories.tsx(1 hunks)src/components/presentational/Action/Action.tsx(2 hunks)src/components/presentational/ActivityMeter/ActivityMeter.stories.tsx(2 hunks)src/components/presentational/ActivityMeter/ActivityMeter.tsx(1 hunks)src/components/presentational/Button/Button.stories.tsx(2 hunks)src/components/presentational/Button/Button.tsx(1 hunks)src/components/presentational/Calendar/Calendar.stories.tsx(2 hunks)src/components/presentational/Calendar/Calendar.tsx(1 hunks)src/components/presentational/CardTitle/CardTitle.stories.tsx(1 hunks)src/components/presentational/CardTitle/CardTitle.tsx(1 hunks)src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx(1 hunks)src/components/presentational/DateRangeNavigator/DateRangeNavigator.tsx(2 hunks)src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.stories.tsx(1 hunks)src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.tsx(1 hunks)src/components/presentational/DumbbellChart/DumbbellChart.stories.tsx(1 hunks)src/components/presentational/DumbbellChart/DumbbellChart.tsx(1 hunks)src/components/presentational/Face/Face.stories.tsx(1 hunks)src/components/presentational/Face/Face.tsx(1 hunks)src/components/presentational/Histogram/Histogram.stories.tsx(1 hunks)src/components/presentational/Histogram/Histogram.tsx(1 hunks)src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.stories.tsx(1 hunks)src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.tsx(1 hunks)src/components/presentational/Layout/Layout.stories.tsx(2 hunks)src/components/presentational/Layout/Layout.tsx(1 hunks)src/components/presentational/LoadingIndicator/LoadingIndicator.stories.tsx(1 hunks)src/components/presentational/Section/Section.stories.tsx(1 hunks)src/components/presentational/Section/Section.tsx(1 hunks)src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx(2 hunks)src/components/presentational/SegmentedControl/SegmentedControl.tsx(1 hunks)src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx(1 hunks)src/components/presentational/SingleExternalAccount/SingleExternalAccount.tsx(1 hunks)src/components/presentational/SingleNotification/SingleNotification.stories.tsx(1 hunks)src/components/presentational/SingleNotification/SingleNotification.tsx(1 hunks)src/components/presentational/SparkBarChart/SparkBarChart.stories.tsx(1 hunks)src/components/presentational/SparkBarChart/SparkBarChart.tsx(2 hunks)src/components/presentational/Switch/Switch.stories.tsx(1 hunks)src/components/presentational/Switch/Switch.tsx(2 hunks)src/components/presentational/TrackerItem/TrackerItem.stories.tsx(1 hunks)src/components/presentational/TrackerItem/TrackerItem.tsx(1 hunks)src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx(3 hunks)src/components/presentational/WeekCalendar/WeekCalendar.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/components/presentational/SingleNotification/SingleNotification.tsx
- src/components/presentational/Action/Action.tsx
- src/components/presentational/Calendar/Calendar.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx
[error] 56-56: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
src/components/presentational/DumbbellChart/DumbbellChart.tsx
[error] 12-12: Shouldn't redeclare 'Dumbbell'. Consider to delete it or rename it.
'Dumbbell' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (29)
src/components/presentational/SingleNotification/SingleNotification.stories.tsx (1)
8-16: Well-structured meta configuration!The meta configuration follows current Storybook best practices with proper typing and clear structure.
src/components/presentational/WeekCalendar/WeekCalendar.tsx (3)
3-3: LGTM! Import statements are properly organized.The semicolon addition maintains consistent code style.
20-20: Great improvement! Named function export enhances debugging and DevTools experience.Converting the anonymous function to a named function export improves:
- Stack trace readability during debugging
- Component identification in React DevTools
- Code maintainability
Line range hint
9-18: LGTM! TypeScript types are well-defined.The interface definition is comprehensive and properly typed with appropriate optional properties.
src/components/presentational/SingleExternalAccount/SingleExternalAccount.tsx (1)
18-18: LGTM! Good improvement in component naming.The change from an anonymous function to a named function enhances debuggability and follows React best practices.
src/components/presentational/Section/Section.stories.tsx (1)
1-4: Verify component imports and type definitionsLet's ensure all required types are properly imported and the components are available.
✅ Verification successful
All component imports and type definitions are valid
The imports in Section.stories.tsx correctly reference existing components and their type definitions. Each imported component (Section, Action, and Layout) exists with proper default exports and type definitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Section component exports proper types ast-grep --pattern 'export interface $interface { $$$ }' src/components/presentational/Section/Section.tsx # Verify Section component's default export ast-grep --pattern 'export default $component' src/components/presentational/Section/Section.tsxLength of output: 209
Script:
#!/bin/bash # Verify file existence and show content fd -e tsx "Section.tsx" --exec cat {} # Check imports in related files echo "=== Action Component ===" fd -e tsx "Action.tsx" --exec cat {} echo "=== Layout Component ===" fd -e tsx "Layout.tsx" --exec cat {}Length of output: 11097
src/components/presentational/Button/Button.tsx (1)
22-22: LGTM! Named export improves debugging.The addition of the function name 'Button' to the default export improves stack traces and follows React best practices.
src/components/presentational/TrackerItem/TrackerItem.stories.tsx (1)
17-19: LGTM! Clean render function implementationGood job extracting the render logic into a reusable function with proper TypeScript typing.
src/components/presentational/Switch/Switch.tsx (1)
13-13: LGTM! Named function exportGood job adding a name to the exported function, which improves stack traces and debugging.
src/components/presentational/Switch/Switch.stories.tsx (1)
19-25: LGTM! Well-structured render functionThe render function is well-organized, providing consistent layout and styling across stories.
src/components/presentational/Section/Section.tsx (1)
15-15: LGTM! Improved component namingGood improvement - adding a name to the default export function enhances code clarity and debugging.
src/components/presentational/Face/Face.stories.tsx (1)
17-21: LGTM! Clean render function implementationThe render function provides a consistent layout wrapper for all stories.
src/components/presentational/TrackerItem/TrackerItem.tsx (1)
17-17: LGTM! Named export improves debuggingThe change from anonymous to named export function improves stack traces and debugging experience while maintaining type safety.
src/components/presentational/Face/Face.tsx (1)
19-19: LGTM! Named export improves debuggingThe change from anonymous to named export function improves stack traces and debugging experience while maintaining type safety.
src/components/presentational/Histogram/Histogram.stories.tsx (1)
6-38: LGTM! Story format updated to latest Storybook standardsThe changes correctly implement the new Storybook format using Meta and StoryObj types, improving type safety and maintainability.
src/components/presentational/SegmentedControl/SegmentedControl.tsx (1)
16-16: LGTM! Named export improves debuggingThe change from anonymous to named export function improves stack traces and debugging experience while maintaining type safety.
src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx (1)
17-19: Well-structured render function implementationGood job extracting the common render logic into a reusable function. This promotes code reuse and maintains consistency across stories.
src/components/presentational/Button/Button.stories.tsx (1)
Line range hint
19-27: Well-structured story implementationGood job on:
- Extracting common render logic
- Wrapping button in appropriate layout components
- Consistent story structure
src/components/presentational/Layout/Layout.stories.tsx (3)
6-6: LGTM! Clean migration to modern Storybook types.The update from ComponentMeta/ComponentStory to Meta/StoryObj follows the latest Storybook best practices.
Also applies to: 8-8, 16-17
Line range hint
19-29: Well-structured render function with clear component hierarchy.The render function provides a reusable template that maintains consistency across stories.
30-72: Clean migration to object-style story definitions.The story definitions are well-organized and properly typed, improving maintainability.
src/components/presentational/ActivityMeter/ActivityMeter.tsx (1)
21-21: LGTM! Named export improves debugging.Adding the name 'ActivityMeter' to the default export function enhances component identification in stack traces and developer tools.
src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.stories.tsx (2)
5-6: LGTM! Improved type safety with modern Storybook patterns.The migration to Meta/StoryObj and explicit typing of TermInformationReference enhances type safety.
Also applies to: 8-14, 16-17
19-23: Clean render function implementation.The render function provides a clear template with proper component hierarchy.
src/components/presentational/DateRangeNavigator/DateRangeNavigator.tsx (1)
20-20: LGTM! Named export improves component identification.Adding the name 'DateRangeNavigator' to the default export function enhances debugging capabilities.
src/components/presentational/DumbbellChart/DumbbellChart.stories.tsx (2)
31-31: Verify data point ranges against axis constraintsSeveral data points contain values that fall outside the defined axis range (_minDiastolic=0, _maxSystolic=250):
- Line 31: Values [-50, 25] are below the minimum
- Line 38: Value -405 is significantly below the minimum
- Lines 39-41: Values 300-350 exceed the maximum of 250
This may cause rendering issues or incorrect visualization.
Consider either:
- Adjusting the axis range to accommodate these values, or
- Clamping the values to the defined range
Also applies to: 38-41
17-20: LGTM: Story format updated correctlyThe story format has been properly updated to use the new
@storybook/reacttypes:
- Correct usage of
MetaandStoryObj- Proper type inference with
typeof DumbbellChart- Clean render function implementation with Layout wrapper
Also applies to: 27-45
src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx (1)
42-44: LGTM: Story definitions updated correctlyThe story definitions have been properly updated to use the new Storybook format with proper TypeScript types.
Also applies to: 100-102
src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.tsx (1)
30-30: LGTM: Named export improves debuggingAdding the name to the default exported function improves stack traces and debugging experience.
src/components/presentational/SingleNotification/SingleNotification.stories.tsx
Outdated
Show resolved
Hide resolved
src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx
Outdated
Show resolved
Hide resolved
src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx
Outdated
Show resolved
Hide resolved
src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx (1)
38-38:⚠️ Potential issueFix CSS variable reference syntax
The CSS variable reference syntax is incorrect.
- color: "--var(--mdhui-color-primary)", + color: "var(--mdhui-color-primary)",
🧹 Nitpick comments (7)
src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx (2)
21-21: Replace var with constUse const instead of var for better scoping and to prevent accidental reassignment.
-var currentDate = new Date(); +const currentDate = new Date();
23-61: Consider centralizing date initialization logicThe date initialization logic is repeated across stories. Consider extracting it into helper functions:
const getStartOfDay = () => { const date = new Date(); return new Date(date.getFullYear(), date.getMonth(), date.getDate(), 0, 0, 0, 0); }; const getStartOfMonth = () => { const date = new Date(); return new Date(date.getFullYear(), date.getMonth(), 1, 0, 0, 0, 0); };src/components/presentational/Layout/Layout.stories.tsx (2)
Line range hint
19-29: Consider enhancing the render function's type safety and reusability.The render function could be more flexible and type-safe.
Consider this improvement:
-const render = (args: LayoutProps) => <Layout {...args}> +const render = (args: LayoutProps & { children?: React.ReactNode }) => <Layout {...args}> - <TextBlock>Layouts</TextBlock> - <Card> - <TextBlock>The layout component should be used as a wrapper around your - MyDataHelps view. It will set some global styles and provide some theming - capabilities. You may control the color scheme by passing in the - colorScheme prop. - </TextBlock> - </Card> + {args.children || ( + <> + <TextBlock>Layouts</TextBlock> + <Card> + <TextBlock>The layout component should be used as a wrapper around your + MyDataHelps view. It will set some global styles and provide some theming + capabilities. You may control the color scheme by passing in the + colorScheme prop. + </TextBlock> + </Card> + </> + )} </Layout>;
30-72: LGTM! Comprehensive story coverage with consistent patterns.The stories effectively demonstrate various Layout configurations. Consider adding JSDoc comments to document the purpose of each story variation.
Add documentation like this:
+/** + * Default story showing automatic color scheme adaptation based on system preferences. + */ export const AutoColorScheme: Story = { args: { colorScheme: "auto" }, render: render };src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.stories.tsx (1)
21-39: Consider documenting color array purpose.While the implementation is correct, consider adding JSDoc comments to explain the purpose and expected format of primaryColors and secondaryColors arrays.
src/components/presentational/Section/Section.stories.tsx (1)
16-22: Consider extracting Action props to a constant.While the render function is well-structured, consider extracting the hardcoded Action props to a constant for better maintainability.
+const defaultActionProps = { + title: "Baseline Survey", + subtitle: "Tap here to start your baseline survey" +}; const render = (args: SectionProps) => <Layout colorScheme="auto"> <Section {...args}> - <Action title="Baseline Survey" subtitle="Tap here to start your baseline survey" /> + <Action {...defaultActionProps} /> </Section> </Layout>;src/components/presentational/SingleNotification/SingleNotification.stories.tsx (1)
25-41: Consider type safety improvements for notification object.While functional, consider extracting the notification object type and using it for better type safety.
type NotificationContent = { title: string; }; type Notification = { id: string; identifier: string; sentDate: string; statusCode: string; type: string; content: NotificationContent; recipients: unknown[]; contentVersion: number; }; const defaultNotification: Notification = { id: "4", identifier: "SurveyReminder", sentDate: addDays(new Date(), -1).toISOString(), statusCode: "Succeeded", type: "Push", content: { title: "Have you received your Fitbit? Connect it!" }, recipients: [], contentVersion: 1 }; export const Default: Story = { args: { notification: defaultNotification }, render: render };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/components/container/ConnectFitbit/ConnectFitbit.stories.tsx(1 hunks)src/components/presentational/Action/Action.stories.tsx(1 hunks)src/components/presentational/ActivityMeter/ActivityMeter.stories.tsx(2 hunks)src/components/presentational/Button/Button.stories.tsx(2 hunks)src/components/presentational/Calendar/Calendar.stories.tsx(2 hunks)src/components/presentational/CardTitle/CardTitle.stories.tsx(1 hunks)src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx(1 hunks)src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.stories.tsx(1 hunks)src/components/presentational/DumbbellChart/Dumbbell.tsx(1 hunks)src/components/presentational/DumbbellChart/DumbbellChart.tsx(1 hunks)src/components/presentational/Face/Face.stories.tsx(1 hunks)src/components/presentational/Histogram/Histogram.stories.tsx(1 hunks)src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.stories.tsx(1 hunks)src/components/presentational/Layout/Layout.stories.tsx(2 hunks)src/components/presentational/LoadingIndicator/LoadingIndicator.stories.tsx(1 hunks)src/components/presentational/Section/Section.stories.tsx(1 hunks)src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx(2 hunks)src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx(1 hunks)src/components/presentational/SingleNotification/SingleNotification.stories.tsx(1 hunks)src/components/presentational/SparkBarChart/SparkBarChart.stories.tsx(1 hunks)src/components/presentational/Switch/Switch.stories.tsx(1 hunks)src/components/presentational/TrackerItem/TrackerItem.stories.tsx(1 hunks)src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/container/ConnectFitbit/ConnectFitbit.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- src/components/presentational/LoadingIndicator/LoadingIndicator.stories.tsx
- src/components/presentational/Calendar/Calendar.stories.tsx
- src/components/presentational/ActivityMeter/ActivityMeter.stories.tsx
- src/components/presentational/Histogram/Histogram.stories.tsx
- src/components/presentational/WeekCalendar/WeekCalendar.stories.tsx
- src/components/presentational/SingleExternalAccount/SingleExternalAccount.stories.tsx
- src/components/presentational/LabResultWithSparkline/LabResultWithSparkline.stories.tsx
- src/components/presentational/CardTitle/CardTitle.stories.tsx
- src/components/presentational/Face/Face.stories.tsx
- src/components/presentational/DumbbellChart/DumbbellChart.tsx
🔇 Additional comments (19)
src/components/presentational/TrackerItem/TrackerItem.stories.tsx (3)
4-14: LGTM! Clean implementation of the new Storybook configuration.The migration to
MetaandStoryObjtypes is correctly implemented, with proper typing and configuration.
17-19: LGTM! Well-structured render function with proper typing.The render function correctly handles component props and maintains proper type safety.
21-30: LGTM! Story configuration follows the new format.The story is well-structured with proper typing and clear prop values.
src/components/presentational/Switch/Switch.stories.tsx (4)
2-14: LGTM! Clean implementation of imports and meta configuration.The file structure and meta configuration follow the new Storybook conventions properly.
19-25: LGTM! Well-structured render function with proper component composition.The render function provides a clean implementation with proper component hierarchy.
27-32: LGTM! Clean implementation of the Default story.The story follows the new format with proper typing and minimal, sufficient props.
34-40: LGTM! Good demonstration of component customization.The CustomBackground story effectively shows the component's styling capabilities.
src/components/presentational/SparkBarChart/SparkBarChart.stories.tsx (1)
6-57: Implementation follows Storybook best practices!The story implementation is well-structured with:
- Proper TypeScript types using the new Storybook API
- Consistent rendering approach using Layout wrapper
- Good test data coverage with varied bar fills and colors
src/components/presentational/Action/Action.stories.tsx (1)
9-63: Well-structured story implementation!The implementation demonstrates good practices:
- Proper story organization with clear variants
- Consistent rendering approach
- Good use of icons and className customization
src/components/presentational/Button/Button.stories.tsx (1)
Line range hint
8-95: Excellent story implementation with comprehensive variants!The implementation demonstrates best practices:
- Complete coverage of button states and variants
- Consistent rendering approach with proper component hierarchy
- Well-structured props for each variant
src/components/presentational/Layout/Layout.stories.tsx (1)
6-16: LGTM! Clean migration to modern Storybook configuration.The migration from ComponentMeta to Meta follows the latest Storybook best practices.
src/components/presentational/DumbbellChart/Dumbbell.tsx (2)
6-6: LGTM! Type change is correctly implemented.The type change from
DumbbelltoDumbbellDataPointis properly reflected in the interface and the component's implementation correctly accesses properties through the new type structure.
2-2: Verify type definitions in DumbbellChart.ts(x).The imported types should be defined in the referenced file.
✅ Verification successful
Import types verified successfully
All imported types (
DumbbellDataPoint,DumbbellClass, andAxis) are properly defined and exported in the sibling fileDumbbellChart.tsx. The imports are correct and the types are used consistently throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the imported types exist in the DumbbellChart file # Expected: Find type definitions for DumbbellDataPoint, DumbbellClass, and Axis ast-grep --pattern 'export type DumbbellDataPoint = $_' || \ ast-grep --pattern 'export interface DumbbellDataPoint $_' ast-grep --pattern 'export enum DumbbellClass $_' || \ ast-grep --pattern 'export type DumbbellClass = $_' ast-grep --pattern 'export type Axis = $_' || \ ast-grep --pattern 'export interface Axis $_'Length of output: 154
Script:
#!/bin/bash # First, find the DumbbellChart file fd DumbbellChart.ts -t f fd DumbbellChart.tsx -t f # Then check its contents for type definitions using broader patterns ast-grep --pattern 'type DumbbellDataPoint' ast-grep --pattern 'interface DumbbellDataPoint' ast-grep --pattern 'type DumbbellClass' ast-grep --pattern 'interface DumbbellClass' ast-grep --pattern 'enum DumbbellClass' ast-grep --pattern 'type Axis' ast-grep --pattern 'interface Axis' # Fallback to ripgrep for additional context rg "DumbbellDataPoint|DumbbellClass|Axis" -A 2Length of output: 28420
src/components/presentational/DayTrackerSymbol/DayTrackerSymbol.stories.tsx (2)
6-14: LGTM! Clean meta configuration.The meta configuration follows the new Storybook format correctly, with proper typing and component configuration.
15-19: LGTM! Well-structured story setup.The Story type is correctly defined, and the render function properly handles component props within a Layout wrapper.
src/components/presentational/Section/Section.stories.tsx (2)
7-15: LGTM! Properly configured meta object.The meta configuration is correctly typed for the Section component, resolving previous issues noted in past reviews.
24-40: LGTM! Well-organized story variants.The story variants effectively demonstrate different Section configurations with proper prop usage.
src/components/presentational/SingleNotification/SingleNotification.stories.tsx (2)
8-16: LGTM! Clean meta configuration.The meta configuration follows the new Storybook format correctly.
19-23: LGTM! Well-structured render function.The render function properly wraps the SingleNotification component within a Card and Layout.
src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx
Outdated
Show resolved
Hide resolved
| @@ -1,33 +1,38 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
I'm not sure if it matters, but some presentational components aren't included here. GlucoseStats and Grid for example.
There was a problem hiding this comment.
Right, and this is bc the story files are not showing as warnings or errors when running npm rollup or npx tsc --noEmit
There was a problem hiding this comment.
Really? That's surprising since they seemed to use some of the same things you were fixing here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/presentational/Action/Action.stories.tsx (1)
41-43: 🛠️ Refactor suggestionConsider using theme colors for better accessibility
The hardcoded color values (#2e6e9e) might not provide sufficient contrast across different themes. Consider using theme-aware colors.
- args: {...baseArgs, icon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />}, + args: {...baseArgs, icon: <FontAwesomeSvgIcon icon={faFile} className="theme-color-primary" />},This aligns with the previous reviewer's comment about using standard colors from our theme.
🧹 Nitpick comments (4)
src/components/presentational/CardTitle/CardTitle.stories.tsx (2)
18-23: Consider extracting card wrapper to improve reusabilityThe render function currently includes a Card wrapper that might be better as a separate component if it's used across multiple stories.
+const CardWrapper: React.FC<{children: React.ReactNode}> = ({children}) => ( + <Card> + {children} + <div style={{ padding: "16px" }}>Card Content</div> + </Card> +); -const render = (args: CardTitleProps) => <Layout colorScheme="auto"> - <Card> - <CardTitle {...args} /> - <div style={{ padding: "16px" }}>Card Content</div> - </Card> -</Layout>; +const render = (args: CardTitleProps) => ( + <Layout colorScheme="auto"> + <CardWrapper> + <CardTitle {...args} /> + </CardWrapper> + </Layout> +);
25-29: Consider using more descriptive alert messageThe alert message could be more descriptive to help users understand which action triggered it.
- onDetailClick: () => { alert("Detail Link Clicked") } + onDetailClick: () => { alert("CardTitle: Detail link was clicked") }src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx (2)
21-21: Standardize date initialization across storiesDifferent stories use varying approaches to initialize dates. Consider standardizing the approach:
- Day story uses
currentDatedirectly- Other stories reconstruct the date with varying components
Consider creating utility functions for date initialization:
const getStartOfDay = (date: Date) => new Date(date.getFullYear(), date.getMonth(), date.getDate(), 0, 0, 0, 0); const getStartOfMonth = (date: Date) => new Date(date.getFullYear(), date.getMonth(), 1, 0, 0, 0, 0); // Then use in stories: intervalStart: getStartOfDay(currentDate), // for Day and Week intervalStart: getStartOfMonth(currentDate), // for Month and SixMonthAlso applies to: 33-33, 41-41, 49-49, 57-57
21-21: Consider using const for currentDateUsing
varfor currentDate is outdated. Consider usingconstsince the value isn't reassigned.-var currentDate = new Date(); +const currentDate = new Date();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/presentational/Action/Action.stories.tsx(1 hunks)src/components/presentational/CardTitle/CardTitle.stories.tsx(1 hunks)src/components/presentational/CardTitle/CardTitle.tsx(1 hunks)src/components/presentational/DateRangeNavigator/DateRangeNavigator.stories.tsx(1 hunks)src/components/presentational/Section/Section.stories.tsx(1 hunks)src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/presentational/CardTitle/CardTitle.tsx
- src/components/presentational/Section/Section.stories.tsx
- src/components/presentational/SegmentedControl/SegmentedControl.stories.tsx
🔇 Additional comments (1)
src/components/presentational/CardTitle/CardTitle.stories.tsx (1)
5-5: LGTM! Clean migration to new Storybook formatThe migration from ComponentMeta to Meta and the meta configuration looks correct. The layout parameter is properly configured.
Also applies to: 7-15
| export const WithTitleIcon: Story = { | ||
| args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />}, | ||
| render: render |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency in icon color usage
For consistency with the previous story, the titleIcon should use the same color approach.
- args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />},
+ args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} className="theme-color-primary" />},📝 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.
| export const WithTitleIcon: Story = { | |
| args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />}, | |
| render: render | |
| export const WithTitleIcon: Story = { | |
| args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} className="theme-color-primary" />}, | |
| render: render |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/presentational/Section/Section.stories.tsx (2)
17-23: Consider making the Section content more descriptive.While the render function correctly implements the suggested layout with content above and below, the static "Section" text could be more descriptive or dynamic based on the story context.
const render = (args: SectionProps) => <Layout colorScheme="auto"> <div>Content above section</div> <Section {...args}> - <div>Section </div> + <div>{args.children || "Default Section Content"}</div> </Section> <div>Content below section</div> </Layout>;
25-37: Consider adding explicit args to the Default story.While the stories effectively demonstrate different Section configurations, the Default story could benefit from explicit args to better showcase basic usage.
export const Default: Story = { + args: { + children: <div>This is a basic section that demonstrates default styling and spacing</div> + }, render: render };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/presentational/Action/Action.stories.css(1 hunks)src/components/presentational/Section/Section.stories.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/presentational/Action/Action.stories.css
🔇 Additional comments (2)
src/components/presentational/Section/Section.stories.tsx (2)
1-14: LGTM! Clean imports and proper meta configuration.The imports are well-organized, and the meta configuration follows the new Storybook format correctly.
15-15: LGTM! Correct Story type definition.The Story type is properly defined using StoryObj with the correct component type.
This PR corrects all typescript errors in the presentational components. These can be seen when running rollup scripts, and hinder being able to add typescript compilation to the dev pipeline.
These changes are mostly for updating stories to the new story format, and cleaning up any issues found while testing.
Security
REMINDER: All file contents are public.
Describe briefly what security risks you considered, why they don't apply, or how they've been mitigated.
Checklist
Testing
Documentation
Consider "Squash and merge" as needed to keep the commit history reasonable on
main.Reviewers
Assign to the appropriate reviewer(s). Minimally, a second set of eyes is needed ensure no non-public information is published. Consider also including: