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.

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

Hackathon - Heart Essential 8 #289

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

reneefromhold
Copy link
Collaborator

@reneefromhold reneefromhold commented Aug 8, 2024

Overview

Heart Essential 8 component.

Mobile friendly view. Hoping to get some eyes on for next steps.

Security

REMINDER: All file contents are public.

  • I have ensured no secure credentials or sensitive information remain in code, metadata, comments, etc.
    • Please verify that you double checked that .storybook/preview.js does not contain your participant access key details.
    • There are no temporary testing changes committed such as API base URLs, access tokens, print/log statements, etc.
  • These changes do not introduce any security risks, or any such risks have been properly mitigated.

Describe briefly what security risks you considered, why they don't apply, or how they've been mitigated.

Checklist

Testing

Documentation

  • I have added relevant Storybook updates to this PR as well.
  • If this feature requires a developer doc update, tag @CareEvolution/api-docs.

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:

  • Subject-matter experts
  • Style/editing reviewers
  • Others requested by the content owner

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced the Heart Essential Eight component to display health metrics related to heart health, including Activity, Blood Pressure, and more.
    • Added visually engaging styles for the Heart Essential Eight component to enhance user experience.
    • Implemented a preview dataset for health metrics, facilitating easier integration into the component.
  • Documentation

    • Created Storybook stories for the Heart Essential Eight component, demonstrating various states and usage scenarios.

Copy link
Contributor

coderabbitai bot commented Aug 8, 2024

Walkthrough

The recent updates introduce the HeartEssentialEight component, including its CSS, preview data, and Storybook configurations, focusing on essential heart health metrics. The CSS enhances layout and presentation, while the main component effectively manages scores and achievements for metrics such as Activity and Blood Pressure. Additionally, Storybook stories document various component states, improving usability and testing capabilities.

Changes

Files Change Summary
src/components/container/HeartEssentialEight/*.css Introduced new CSS classes for styling the HeartEssentialEight component, enhancing layout, score display, and feedback sections.
src/components/container/HeartEssentialEight/*.previewdata.tsx Added a previewHeartEssentialData object for standardized preview health metrics, facilitating component integration.
src/components/container/HeartEssentialEight/*.stories.tsx Created Storybook stories for HeartEssentialEight, defining Default, NoData, and Live scenarios to visually demonstrate different states.
src/components/container/HeartEssentialEight/*.tsx Developed the main React component to manage and display health metrics, incorporating data fetching, state management, and achievement mapping.
src/components/container/HeartEssentialEight/index.ts Added an entry point for exporting the HeartEssentialEight component, simplifying import paths.
src/components/container/index.ts Enhanced module exports by making the HeartEssentialEight component available for import in other parts of the application.

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 10a5607 and 3efda8d.

Files ignored due to path filters (8)
  • src/components/container/HeartEssentialEight/icons/controlcholesterol.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/eatwell.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/getactive.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/loseweight.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/managebloodpressure.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/reducebloodsugar.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/sleep.svg is excluded by !**/*.svg
  • src/components/container/HeartEssentialEight/icons/stopsmoking.svg is excluded by !**/*.svg
Files selected for processing (3)
  • src/components/container/HeartEssentialEight/HeartEssentialEight.css (1 hunks)
  • src/components/container/HeartEssentialEight/HeartEssentialEight.stories.tsx (1 hunks)
  • src/components/container/HeartEssentialEight/HeartEssentialEight.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/components/container/HeartEssentialEight/HeartEssentialEight.css
  • src/components/container/HeartEssentialEight/HeartEssentialEight.stories.tsx
Additional context used
Biome
src/components/container/HeartEssentialEight/HeartEssentialEight.tsx

[error] 183-183: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

Additional comments not posted (7)
src/components/container/HeartEssentialEight/HeartEssentialEight.tsx (7)

1-14: Imports look good.

The import statements are well-organized and relevant to the component's functionality.


16-32: Type definitions are clear and appropriate.

The HeartEssentialEightScores and HeartEssentialEightProps types are well-defined and enhance type safety.


34-44: Use const for state variables.

The use of let for score and achievementMap is unconventional. Consider using const as these variables are managed by React state hooks.


45-56: Add error handling to async operations.

Consider adding error handling to the MyDataHelps.getParticipantInfo() promise to manage potential errors gracefully.


59-85: Improve type safety for newAchievementMap.

Consider initializing newAchievementMap with a more explicit type to enhance type safety.


87-168: Enhance type safety and readability.

Consider using more specific types for map and achievementList to enhance type safety and readability.


172-215: Avoid using the children prop directly in JSX.

The ProgressRing component should use JSX syntax to pass children, as recommended by React best practices.

Tools
Biome

[error] 183-183: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

@reneefromhold reneefromhold marked this pull request as draft August 8, 2024 19:53
@reneefromhold reneefromhold marked this pull request as draft August 8, 2024 19:53
Copy link
Contributor

@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: 3

Outside diff range, codebase verification and nitpick comments (2)
src/components/container/HeartEssentialEight/HeartEssentialEight.stories.tsx (1)

1-62: LGTM! Consider improving readability.

The Storybook stories for HeartEssentialEight are well-structured and comprehensive. Consider using a loop or a helper function to populate customFieldDictionary if the list grows, to enhance maintainability.

src/components/container/HeartEssentialEight/HeartEssentialEight.css (1)

43-45: Remove outdated vendor prefixes.

The -moz-border-radius and -webkit-border-radius prefixes are no longer necessary for modern browsers.

-  -moz-border-radius: 50px;
-  -webkit-border-radius: 50px;
  border-radius: 50px;

Also applies to: 63-65

Comment on lines 34 to 184
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}

const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);

const achievement = (
<Action key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
);
achievementList.push(achievement);
}
}

return achievementList;
}

useInitializeView(initialize, [], [props.previewState]);

return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>

</>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using the children prop directly in JSX.

The ProgressRing component should use JSX syntax to pass children, as recommended by React best practices.

- <ProgressRing
-   percentCompleted={score}
-   color="rgb(193, 14, 33)"
-   children={
-     <div className="scoreContainer">
-       <div className="score">{score}</div>
-       <div className="scoreSubText">out of 100</div>
-     </div>
-   }
-   animate={true}
- />
+ <ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
+   <div className="scoreContainer">
+     <div className="score">{score}</div>
+     <div className="scoreSubText">out of 100</div>
+   </div>
+ </ProgressRing>
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 default function (props: HeartEssentialEightProps) {
const { customFieldDictionary } = props;
let [score, setScore] = React.useState<number>();
let [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();
async function initialize() {
if (props.previewState) {
if (["Default", "NoData"].includes(props.previewState ?? "")) {
const data =
props.previewState === "Default" ? previewHeartEssentialData : {};
loadScoresFromCustomFieldData(data);
} else {
MyDataHelps.getParticipantInfo().then((participantInfo) => {
loadScoresFromCustomFieldData(participantInfo.customFields);
});
}
}
}
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;
for (const key in customFieldDictionary) {
if (customFieldDictionary[key]) {
let surveyScore = customFieldData[customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}
if (newScore > 0){
newScore = (newScore / 800) * 100;
}
setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}
const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);
const achievement = (
<Action key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
);
achievementList.push(achievement);
}
}
return achievementList;
}
useInitializeView(initialize, [], [props.previewState]);
return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>
</>
);
}
return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
</ProgressRing>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>
</>
);
Tools
Biome

[error] 165-165: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

@reneefromhold reneefromhold changed the title Hackathon Draft - Heart Essential 8 Hackathon - Heart Essential 8 Aug 9, 2024
@reneefromhold reneefromhold marked this pull request as ready for review August 9, 2024 21:40
Copy link
Contributor

@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: 5

Comment on lines 40 to 51
async function initialize() {
if (props.previewState) {
if (["Default", "NoData"].includes(props.previewState ?? "")) {
const data =
props.previewState === "Default" ? previewHeartEssentialData : {};
loadScoresFromCustomFieldData(data);
}
} else {
MyDataHelps.getParticipantInfo().then((participantInfo) => {
loadScoresFromCustomFieldData(participantInfo.customFields);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling to async operations.

Consider adding error handling to the MyDataHelps.getParticipantInfo() promise to manage potential errors gracefully.

MyDataHelps.getParticipantInfo()
  .then((participantInfo) => {
    loadScoresFromCustomFieldData(participantInfo.customFields);
  })
  .catch((error) => {
    console.error("Failed to fetch participant info:", error);
  });

Comment on lines 34 to 38
export default function (props: HeartEssentialEightProps) {
const { customFieldDictionary } = props;
let [score, setScore] = React.useState<number>();
let [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const for state variables.

The use of let for score and achievementMap is unconventional. Consider using const as these variables are managed by React state hooks.

- let [score, setScore] = React.useState<number>();
- let [achievementMap, setAchievementMap] = useState<Record<HeartEssentialEightScores, boolean>>();
+ const [score, setScore] = React.useState<number>();
+ const [achievementMap, setAchievementMap] = useState<Record<HeartEssentialEightScores, boolean>>();
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 default function (props: HeartEssentialEightProps) {
const { customFieldDictionary } = props;
let [score, setScore] = React.useState<number>();
let [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();
export default function (props: HeartEssentialEightProps) {
const { customFieldDictionary } = props;
const [score, setScore] = React.useState<number>();
const [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();

Comment on lines 157 to 184
return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>

</>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using the children prop directly in JSX.

The ProgressRing component should use JSX syntax to pass children, as recommended by React best practices.

- <ProgressRing
-   percentCompleted={score}
-   color="rgb(193, 14, 33)"
-   children={
-     <div className="scoreContainer">
-       <div className="score">{score}</div>
-       <div className="scoreSubText">out of 100</div>
-     </div>
-   }
-   animate={true}
- />
+ <ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
+   <div className="scoreContainer">
+     <div className="score">{score}</div>
+     <div className="scoreSubText">out of 100</div>
+   </div>
+ </ProgressRing>
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
return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>
</>
);
return (
<>
{/* <LoadingIndicator /> */}
{/* {score === undefined && <LoadingIndicator />} */}
<div className="mdhui-heart-e8-container">
<div className="mdhui-heart-e8-score-container">
<ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
</ProgressRing>
</div>
{achievementMap && <div className="mdhui-heart-e8-feedback">
<div className="achievementSection"><div className="bulletPoint improve" /><h3>IMPROVE</h3></div>
<div className="achievementSection">{generateAchievementMap(false, achievementMap)}</div>
<div className="achievementSection"><div className="bulletPoint celebrate" /><h3>CELEBRATE</h3></div>
<div className="achievementSection">{generateAchievementMap(true, achievementMap)}</div>
</div>}
</div>
</>
);
Tools
Biome

[error] 166-166: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

Comment on lines 55 to 81
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;

for (const key in customFieldDictionary) {
if (customFieldDictionary[key]) {
let surveyScore = customFieldData[customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}

if (newScore > 0){
newScore = (newScore / 800) * 100;
}

setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve type safety for newAchievementMap.

Consider initializing newAchievementMap with a more explicit type to enhance type safety.

- let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;
+ let newAchievementMap: Record<HeartEssentialEightScores, boolean> = {};
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
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;
for (const key in customFieldDictionary) {
if (customFieldDictionary[key]) {
let surveyScore = customFieldData[customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}
if (newScore > 0){
newScore = (newScore / 800) * 100;
}
setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap: Record<HeartEssentialEightScores, boolean> = {};
for (const key in customFieldDictionary) {
if (customFieldDictionary[key]) {
let surveyScore = customFieldData[customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}
if (newScore > 0){
newScore = (newScore / 800) * 100;
}
setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}

Comment on lines 83 to 153
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}

const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);

const achievement = (
<Action key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
);
achievementList.push(achievement);
}
}

return achievementList;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Enhance type safety and readability.

Consider using more specific types for map and achievementList to enhance type safety and readability.

- const map = newAchievementMap as Record<string, boolean>;
- const achievementList: any[] = [];
+ const map: Record<HeartEssentialEightScores, boolean> = newAchievementMap;
+ const achievementList: JSX.Element[] = [];
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
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}
const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);
const achievement = (
<Action key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
);
achievementList.push(achievement);
}
}
return achievementList;
}
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map: Record<HeartEssentialEightScores, boolean> = newAchievementMap;
const achievementList: JSX.Element[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}
const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);
const achievement = (
<Action key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
);
achievementList.push(achievement);
}
}
return achievementList;
}

Copy link
Contributor

@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: 4

Comment on lines +45 to +56
async function initialize() {
if (props.previewState) {
if (["Default", "NoData"].includes(props.previewState ?? "")) {
const data =
props.previewState === "Default" ? previewHeartEssentialData : {};
loadScoresFromCustomFieldData(data);
}
} else {
MyDataHelps.getParticipantInfo().then((participantInfo) => {
loadScoresFromCustomFieldData(participantInfo.customFields);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling to async operations.

Consider adding error handling to the MyDataHelps.getParticipantInfo() promise to manage potential errors gracefully.

MyDataHelps.getParticipantInfo()
  .then((participantInfo) => {
    loadScoresFromCustomFieldData(participantInfo.customFields);
  })
  .catch((error) => {
    console.error("Failed to fetch participant info:", error);
  });

Comment on lines +59 to +85
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;

for (const key in props.customFieldDictionary) {
if (props.customFieldDictionary[key]) {
let surveyScore = customFieldData[props.customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}

if (newScore > 0) {
newScore = (newScore / 800) * 100;
}

setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve type safety for newAchievementMap.

Consider initializing newAchievementMap with a more explicit type to enhance type safety.

- let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;
+ let newAchievementMap: Record<HeartEssentialEightScores, boolean> = {};
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
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap = {} as Record<HeartEssentialEightScores, boolean>;
for (const key in props.customFieldDictionary) {
if (props.customFieldDictionary[key]) {
let surveyScore = customFieldData[props.customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}
if (newScore > 0) {
newScore = (newScore / 800) * 100;
}
setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}
function loadScoresFromCustomFieldData(
customFieldData: Record<string, string>
) {
let newScore: number = 0;
let newAchievementMap: Record<HeartEssentialEightScores, boolean> = {};
for (const key in props.customFieldDictionary) {
if (props.customFieldDictionary[key]) {
let surveyScore = customFieldData[props.customFieldDictionary[key]];
let numericScore = parseInt(surveyScore);
if (!Number.isNaN(numericScore)) {
newScore += numericScore;
newAchievementMap[key as HeartEssentialEightScores] =
numericScore >= 100;
} else {
newAchievementMap[key as HeartEssentialEightScores] = false;
}
}
}
if (newScore > 0) {
newScore = (newScore / 800) * 100;
}
setScore(parseFloat(newScore.toFixed(1)));
setAchievementMap(newAchievementMap);
}

Comment on lines 164 to 204
return (
<>
{score === undefined && <LoadingIndicator />}
{score !== undefined && (
<div>
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>

{achievementMap && (
<>
<div className="achievementSection">
<div className="bulletPoint improve" />
<h4>IMPROVE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(false, achievementMap)}
</div>
<div className="achievementSection">
<div className="bulletPoint celebrate" />
<h4>CELEBRATE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(true, achievementMap)}
</div>
</>
)}
</div>
</div>
)}
</>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using the children prop directly in JSX.

The ProgressRing component should use JSX syntax to pass children, as recommended by React best practices.

- <ProgressRing
-   percentCompleted={score}
-   color="rgb(193, 14, 33)"
-   children={
-     <div className="scoreContainer">
-       <div className="score">{score}</div>
-       <div className="scoreSubText">out of 100</div>
-     </div>
-   }
-   animate={true}
- />
+ <ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
+   <div className="scoreContainer">
+     <div className="score">{score}</div>
+     <div className="scoreSubText">out of 100</div>
+   </div>
+ </ProgressRing>
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
return (
<>
{score === undefined && <LoadingIndicator />}
{score !== undefined && (
<div>
<div className="mdhui-heart-e8-score-container">
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
children={
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
}
animate={true}
/>
{achievementMap && (
<>
<div className="achievementSection">
<div className="bulletPoint improve" />
<h4>IMPROVE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(false, achievementMap)}
</div>
<div className="achievementSection">
<div className="bulletPoint celebrate" />
<h4>CELEBRATE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(true, achievementMap)}
</div>
</>
)}
</div>
</div>
)}
</>
);
return (
<>
{score === undefined && <LoadingIndicator />}
{score !== undefined && (
<div>
<div className="mdhui-heart-e8-score-container">
<ProgressRing percentCompleted={score} color="rgb(193, 14, 33)" animate={true}>
<div className="scoreContainer">
<div className="score">{score}</div>
<div className="scoreSubText">out of 100</div>
</div>
</ProgressRing>
{achievementMap && (
<>
<div className="achievementSection">
<div className="bulletPoint improve" />
<h4>IMPROVE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(false, achievementMap)}
</div>
<div className="achievementSection">
<div className="bulletPoint celebrate" />
<h4>CELEBRATE</h4>
</div>
<div className="achievementIcons">
{generateAchievementMap(true, achievementMap)}
</div>
</>
)}
</div>
</div>
)}
</>
);
Tools
Biome

[error] 173-173: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

Comment on lines 87 to 160
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}

const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);

const achievement = (
<div className="actionRow">
<Action
key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
</div>
);
achievementList.push(achievement);
}
}

return achievementList;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Enhance type safety and readability.

Consider using more specific types for map and achievementList to enhance type safety and readability.

- const map = newAchievementMap as Record<string, boolean>;
- const achievementList: any[] = [];
+ const map: Record<HeartEssentialEightScores, boolean> = newAchievementMap;
+ const achievementList: JSX.Element[] = [];
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
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}
const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);
const achievement = (
<div className="actionRow">
<Action
key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
</div>
);
achievementList.push(achievement);
}
}
return achievementList;
}
function generateAchievementMap(
didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map: Record<HeartEssentialEightScores, boolean> = newAchievementMap;
const achievementList: JSX.Element[] = [];
for (const key in map) {
const vv = map[key];
if (vv === didAchieve) {
const title = key.replace(/([A-Z])/g, " $1").replace(/^./, (str) => {
return str.toUpperCase();
});
let icon: any;
let url: string;
const circleClasses: string[] = ["circle"];
switch (key) {
case "Activity":
icon = iconGetActive;
circleClasses.push("activity");
break;
case "BloodPressure":
icon = iconManageBloodPressure;
circleClasses.push("bloodPressure");
break;
case "BloodSugar":
icon = iconReduceBloodSugar;
circleClasses.push("bloodSugar");
break;
case "BMI":
icon = iconLoseWeight;
circleClasses.push("bmi");
break;
case "Cholesterol":
icon = iconCholesterol;
circleClasses.push("cholesterol");
break;
case "Diet":
icon = iconEatWell;
circleClasses.push("diet");
break;
case "Nicotine":
icon = iconStopSmoking;
circleClasses.push("nicotine");
break;
case "Sleep":
icon = iconSleep;
circleClasses.push("sleep");
break;
default:
icon = null;
}
const achievementIcon = (
<div className={circleClasses.join(" ")}>
<img src={icon} />
</div>
);
const achievement = (
<div className="actionRow">
<Action
key={`achievement-${key}`}
title={title}
titleIcon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
/>
</div>
);
achievementList.push(achievement);
}
}
return achievementList;
}

@n400 n400 mentioned this pull request Aug 20, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this file could use an auto format and some TODO cleanup. Let me know once you are done with the revisions here and I'll take another look at it.

}

export default function (props: HeartEssentialEightProps) {
const customRingStyle: React.CSSProperties = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not appear to be used.

width: "100px",
};

let [score, setScore] = React.useState<number>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can just be useState<number>(). No need for the React prefix.


let [score, setScore] = React.useState<number>();
let [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: Some of this file appears to be rather "narrow" with regard to formatting. We probably don't need to wrap things quite this aggressively.

let [achievementMap, setAchievementMap] =
useState<Record<HeartEssentialEightScores, boolean>>();

async function initialize() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need the async keyword here. No calls to this function are awaiting or expecting a promise.


async function initialize() {
if (props.previewState) {
if (["Default", "NoData"].includes(props.previewState ?? "")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't need the ?? "" here. The includes(...) function should handle an undefined preview state without any issues.

loadScoresFromCustomFieldData(data);
}
} else {
MyDataHelps.getParticipantInfo().then((participantInfo) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: No need for the parens around participantInfo if not specifying a type.

didAchieve: boolean,
newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to do this conversion.

newAchievementMap: Record<HeartEssentialEightScores, boolean>
) {
const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be typed as React.JSX.Element[].

const map = newAchievementMap as Record<string, boolean>;
const achievementList: any[] = [];
for (const key in map) {
const vv = map[key];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use a more meaningful variable name here?

});
let icon: any;
let subtitle: any;
let url: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't look like the URL variable ever gets set. Are we still deciding what to do with these actions when clicked?

title={title}
subtitle={subtitle}
icon={achievementIcon}
onClick={() => MyDataHelps.openExternalUrl(url)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to have this call a handler function on the props. That way, when this is added to the view builder, it can use an ActionDefinition setting to allow the person building the view to configure what happens when each of the types is clicked. Maybe it opens a URL, launches another app, or goes to a different tab, etc.

If we go that route, we'd probably want to support configurable subtitle text for each action as well.

.mdhui-heart-e8-achievement-container .le8-header {
margin: var(--mdhui-padding-sm) 0 var(--mdhui-padding-xs) 0;
}
.achievementActionList {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you will probably want to prefix all these classes with mdhui-heart-e8- to avoid any potential collisions.

<h3 className="le8-header">Heart Health Score</h3>
<ProgressRing
percentCompleted={score}
color="rgb(193, 14, 33)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this use one of the existing color variables?

<>
<Card>
<div className="mdhui-heart-e8-score-container">
<h3 className="le8-header">Heart Health Score</h3>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can use <Title order={3}>Heart Health Score</Title> for these, perhaps with a custom margin, if that remains necessary.

switch (key) {
case "Activity":
icon = iconGetActive;
subtitle="Display Active Time from Fitbit / Garmin. Probably AppleExerciseTime for Apple Health. Or manual entry";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just putting a note here as a reminder to localize all the hard coded text once the design gets finalized.


const render = (args: HeartEssentialEightProps) => {
return (
<Layout colorScheme="auto">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider setting up the story to allow for swapping between color schemes for easier review of light/dark/auto modes.

Similarly, the preview state can be set up as a toggle on a single story if you'd like.

An example: https://github.com/CareEvolution/MyDataHelpsUI/blob/main/src/components/container/ConnectDexcom/ConnectDexcom.stories.tsx

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