Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typescript compilation errors on all presentational components #385

Merged
merged 4 commits into from
Jan 31, 2025
Merged

Fix typescript compilation errors on all presentational components #385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42ff035
Fix typescript compilation errors on all presentational components
reneefromhold Jan 10, 2025
387f5b3
Code rabbit finds
reneefromhold Jan 15, 2025
e087bd1
Feedback from Linda
reneefromhold Jan 17, 2025
dd32e6b
Clean up on colors and stories
reneefromhold Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/container/ConnectFitbit/ConnectFitbit.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import ConnectFitbit, { ConnectFitbitProps } from "./ConnectFitbit"
import Card from "../../presentational/Card"
import Layout from "../../presentational/Layout"
import { Meta, StoryObj } from "@storybook/react/*";
import { Meta, StoryObj } from "@storybook/react";


const meta: Meta<typeof ConnectFitbit> = {
Expand Down
2 changes: 1 addition & 1 deletion src/components/presentational/Action/Action.stories.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
.action-story-primary {
background-color: rosybrown;
background-color: var(--mdhui-color-primary);
}
50 changes: 24 additions & 26 deletions src/components/presentational/Action/Action.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,49 @@
import React from "react";
import { ComponentStory, ComponentMeta } from "@storybook/react";
import Action, { ActionProps } from "./Action";
import Layout from "../Layout"
import Layout from "../Layout";
import './Action.stories.css';
import { FontAwesomeSvgIcon } from "react-fontawesome-svg-icon";
import { faFile } from "@fortawesome/free-solid-svg-icons";
import { Meta, StoryObj } from "@storybook/react";

export default {
const meta: Meta<typeof Action> = {
title: "Presentational/Action",
component: Action,
parameters: {
layout: 'fullscreen',
layout: 'fullscreen'
}
} as ComponentMeta<typeof Action>;
};

const Template: ComponentStory<typeof Action> = (args: ActionProps) =>
export default meta;
type Story = StoryObj<typeof Action>;

const render = (args: ActionProps) =>
<Layout colorScheme="auto">
<Action {...args} />
</Layout>;

export const StartSurvey = Template.bind({});
StartSurvey.args = {
const baseArgs : ActionProps = {
title: "Baseline Survey",
subtitle: "Tap here to start your baseline survey",
onClick: () => alert("Clicked")
};

export const WithClickEvent: Story = {
args: {...baseArgs},
render: render
}

export const WithClass = Template.bind({});
WithClass.args = {
title: "Baseline Survey",
subtitle: "Tap here to start your baseline survey",
className: "action-story-primary",
onClick: () => alert("Clicked")
export const WithClass: Story = {
args: {...baseArgs, className: "action-story-primary", renderAs: "div"},
render: render
}

export const WithIcon = Template.bind({});
WithIcon.args = {
title: "Baseline Survey",
subtitle: "Tap here to start your baseline survey",
icon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />,
onClick: () => alert("Clicked")
export const WithIcon: Story = {
args: {...baseArgs, icon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />},
render: render
}

export const WithTitleIcon = Template.bind({});
WithTitleIcon.args = {
titleIcon: <span><FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />&nbsp;</span>,
title: "Baseline Survey",
subtitle: "Tap here to start your baseline survey",
onClick: () => alert("Clicked")
export const WithTitleIcon: Story = {
args: {...baseArgs, titleIcon: <FontAwesomeSvgIcon icon={faFile} color="#2e6e9e" />},
render: render
Comment on lines +46 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
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

}
4 changes: 2 additions & 2 deletions src/components/presentational/Action/Action.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface ActionProps {
renderAs?: "div" | "button";
}

export default function (props: ActionProps) {
export default function Action(props: ActionProps) {
var indicatorIcon = props.indicatorIcon ?? faChevronRight;

let onClick = props.onClick ?? (() => { });
Expand Down Expand Up @@ -68,7 +68,7 @@ export default function (props: ActionProps) {
if (props.bottomBorder) {
classes.push("mdhui-action-bottom-border");
}
if(props.className){
if (props.className) {
classes.push(props.className);
}
if (props.onClick) {
Expand Down
54 changes: 30 additions & 24 deletions src/components/presentational/ActivityMeter/ActivityMeter.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
import React from "react";
import { ComponentStory, ComponentMeta } from "@storybook/react";
import ActivityMeter, { ActivityMeterProps } from "./ActivityMeter";
import Layout from "../Layout"
import { faShoePrints, faBed } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeSvgIcon } from "react-fontawesome-svg-icon";
import '@fortawesome/fontawesome-svg-core/styles.css';
import Card from "../Card";
import { Meta, StoryObj } from "@storybook/react";

export default {
const meta: Meta<typeof ActivityMeter> = {
title: "Presentational/ActivityMeter",
component: ActivityMeter,
parameters: {
layout: 'fullscreen',
layout: 'fullscreen'
}
} as ComponentMeta<typeof ActivityMeter>;
};

// More on component templates: https://storybook.js.org/docs/react/writing-stories/introduction#using-args
const Template: ComponentStory<typeof ActivityMeter> = (args: ActivityMeterProps) =>
export default meta;
type Story = StoryObj<typeof ActivityMeter>;

const render = (args: ActivityMeterProps) =>
<Layout colorScheme="auto">
<Card>
<div style={{ padding: "16px" }}>
Expand All @@ -25,24 +27,28 @@ const Template: ComponentStory<typeof ActivityMeter> = (args: ActivityMeterProps
</Card>
</Layout>;

export const Sleep = Template.bind({});
Sleep.args = {
fillPercent: .6,
averageFillPercent: .5,
color: "#7b88c6",
icon: <FontAwesomeSvgIcon icon={faBed} />,
label: "Sleep",
value: "8h 5m",
message: "Above average"
export const Sleep: Story = {
args: {
fillPercent: .6,
averageFillPercent: .5,
color: "#7b88c6",
icon: <FontAwesomeSvgIcon icon={faBed} />,
label: "Sleep",
value: "8h 5m",
message: "Above average"
},
render: render
}

export const Steps = Template.bind({});
Steps.args = {
fillPercent: .4,
averageFillPercent: .5,
color: "#f5b722",
icon: <FontAwesomeSvgIcon icon={faShoePrints} rotate={270} />,
label: "Steps",
value: "5,251",
message: "Below average"
export const Steps: Story = {
args: {
fillPercent: .4,
averageFillPercent: .5,
color: "#f5b722",
icon: <FontAwesomeSvgIcon icon={faShoePrints} rotate={270} />,
label: "Steps",
value: "5,251",
message: "Below average"
},
render: render
}
2 changes: 1 addition & 1 deletion src/components/presentational/ActivityMeter/ActivityMeter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface ActivityMeterProps {
thresholdLabel?: string;
}

export default function (props: ActivityMeterProps) {
export default function ActivityMeter(props: ActivityMeterProps) {
let context = useContext(LayoutContext);

return <div ref={props.innerRef} className={"mdhui-activity-meter " + (props.className || "")}>
Expand Down
107 changes: 62 additions & 45 deletions src/components/presentational/Button/Button.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import React from "react";
import { ComponentStory, ComponentMeta } from "@storybook/react";
import Button, { ButtonProps } from "./Button";
import Layout from "../Layout"
import TextBlock from "../TextBlock";
import Section from "../Section";
import { Meta, StoryObj } from "@storybook/react";

export default {
const meta: Meta<typeof Button> = {
title: "Presentational/Button",
component: Button,
parameters: {
// More on Story layout: https://storybook.js.org/docs/react/configure/story-layout
layout: 'fullscreen',
layout: 'fullscreen'
}
} as ComponentMeta<typeof Button>;
};

// More on component templates: https://storybook.js.org/docs/react/writing-stories/introduction#using-args
const Template: ComponentStory<typeof Button> = (args: ButtonProps) =>
export default meta;
type Story = StoryObj<typeof Button>;

const render = (args: ButtonProps) =>
<Layout colorScheme="auto">
<Section>
<TextBlock>
Expand All @@ -24,56 +25,72 @@ const Template: ComponentStory<typeof Button> = (args: ButtonProps) =>
</Section>
</Layout>;

export const Enabled = Template.bind({});
Enabled.args = {
children: "Click Me",
disabled: false
export const Enabled: Story = {
args: {
children: "Click Me",
disabled: false
},
render: render
}

export const Disabled = Template.bind({});
Disabled.args = {
children: "Click Me",
disabled: true
export const Disabled: Story = {
args: {
children: "Click Me",
disabled: true
},
render: render
}

export const CustomColor = Template.bind({});
CustomColor.args = {
disabled: false,
children: "Click Me",
color: "red"
export const CustomColor: Story = {
args: {
disabled: false,
children: "Click Me",
color: "red"
},
render: render
}

export const Loading = Template.bind({});
Loading.args = {
disabled: false,
children: "Doing Something...",
loading: true
export const Loading: Story = {
args: {
disabled: false,
children: "Doing Something...",
loading: true
},
render: render
}

export const SubtleVariant = Template.bind({});
SubtleVariant.args = {
disabled: false,
children: "Click Here",
variant: "subtle"
export const SubtleVariant: Story = {
args: {
disabled: false,
children: "Click Here",
variant: "subtle"
},
render: render
}

export const LightVariant = Template.bind({});
LightVariant.args = {
disabled: false,
children: "Click Here",
variant: "light"
export const LightVariant: Story = {
args: {
disabled: false,
children: "Click Here",
variant: "light"
},
render: render
}

export const NotFullWidth = Template.bind({});
NotFullWidth.args = {
disabled: false,
children: "Click Here",
fullWidth: false
export const NotFullWidth: Story = {
args: {
disabled: false,
children: "Click Here",
fullWidth: false
},
render: render
}

export const DefaultMargin = Template.bind({});
DefaultMargin.args = {
disabled: false,
children: "Click Here",
defaultMargin: true
export const DefaultMargin: Story = {
args: {
disabled: false,
children: "Click Here",
defaultMargin: true
},
render: render
}
4 changes: 2 additions & 2 deletions src/components/presentational/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export interface ButtonProps {
fullWidth?: boolean;
}

export default function (props: ButtonProps) {
export default function Button(props: ButtonProps) {
let classes = ["mdhui-button"];
if (props.className) {
classes.push(props.className);
}
if(props.fullWidth ?? true){
if (props.fullWidth ?? true) {
classes.push("mdhui-button-full-width");
}
Comment on lines +27 to 29
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Confirmed: Button's fullWidth prop needs correction

The current implementation (fullWidth ?? true) contradicts the component's TypeScript interface where fullWidth is defined as an optional boolean. Making buttons full-width by default is unexpected behavior and violates the principle of least surprise.

-if (props.fullWidth ?? true) {
+if (props.fullWidth) {

This change will:

  • Align with the TypeScript interface
  • Make full-width behavior opt-in
  • Follow common UI component patterns where buttons default to natural width
🔗 Analysis chain

Review the default fullWidth behavior.

The current implementation props.fullWidth ?? true makes buttons full width by default, which might not be the expected behavior. Consider:

  1. This contradicts the optional fullWidth?: boolean type in the interface
  2. Typically, buttons should default to their natural width unless explicitly set to full width

Let's check the existing usage to understand the impact:

Consider changing to:

-if (props.fullWidth ?? true) {
+if (props.fullWidth) {
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Button component usage with fullWidth prop
rg "fullWidth[=:]" --type ts --type tsx

Length of output: 69

Script:

#!/bin/bash
# Search for Button component usage and type definitions
rg "fullWidth" -t ts -t typescript -t jsx -t tsx

# Search for Button component definitions and usage patterns
ast-grep --pattern 'interface ButtonProps {
  $$$
}'

# Search for Button usage in JSX
ast-grep --pattern '<Button
  $$$
/>'

Length of output: 1159

Script:

#!/bin/bash
# Search for any Button component imports and usages
rg "Button" -t js -t jsx -t ts

# Try finding Button usages with component pattern
ast-grep --pattern 'Button $$$'

Length of output: 92

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need to leave as is for now

Copy link
Contributor

Choose a reason for hiding this comment

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

@reneefromhold Understood, I'll respect your decision to keep the current behavior. Would you like me to create a GitHub issue to track this as a potential future improvement?

if (props.defaultMargin) {
Expand Down
Loading
Loading