"Basic Points For Badges" Components #262
Conversation
chrisnowak
changed the title
|
Caution Review failedThe pull request is closed. WalkthroughThis update significantly enhances the badge management and presentation system within the application. New components for displaying badges and points have been introduced, accompanied by stories and improved localization support. The update also refines CSS styling across components, integrates Fitbit wear time tracking, and includes interfaces for managing daily data goals. Collectively, these changes enhance both the visual appeal and functionality of the badge tracking system, fostering greater user engagement. Changes
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 as PR comments)
Additionally, you can add 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: 18
Outside diff range and nitpick comments (5)
src/components/presentational/ProgressBar/ProgressBar.tsx (1)
Line range hint
45-45: Avoid using the index as a key in React lists to prevent potential issues with component state and re-rendering.- <div key={`${i}-step`} className="step-icon" style={{ left: Math.trunc(step.percent) + "%" }}> + <div key={`step-${step.percent}`} className="step-icon" style={{ left: `${Math.trunc(step.percent)}%` }}>src/helpers/daily-data-types.tsx (1)
Line range hint
1-2: Consider using TypeScript'simport typesyntax for imports that are only used as types to clarify their usage and potentially optimize bundling.- import { ReactElement } from "react"; - import { DailyDataAvailabilityCheck, DailyDataProvider } from "./query-daily-data"; + import type { ReactElement } from "react"; + import type { DailyDataAvailabilityCheck, DailyDataProvider } from "./query-daily-data";src/helpers/query-daily-data.tsx (3)
Line range hint
36-36: Use template literals for string concatenation to improve readability and consistency.- throw "Unknown data type:" + type; + throw `Unknown data type: ${type}`;Also applies to: 45-45, 81-81
Line range hint
46-56: Convert function expressions to arrow functions for consistency with modern JavaScript practices.- async function randomNumber(message: string) { + const randomNumber = async (message: string) => {- allTypeDefinitions.forEach(function (typeDefinition) { + allTypeDefinitions.forEach((typeDefinition) => {Also applies to: 100-102
Line range hint
35-35: Useletorconstinstead ofvarto adhere to modern JavaScript best practices.- var dailyDataType = dailyDataTypes.get(type); + let dailyDataType = dailyDataTypes.get(type);Also applies to: 44-44, 47-47, 49-49, 60-60, 61-61
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (47)
- package.json (1 hunks)
- src/components/container/BasicBadges/BasicBadges.css (1 hunks)
- src/components/container/BasicBadges/BasicBadges.stories.tsx (1 hunks)
- src/components/container/BasicBadges/BasicBadges.tsx (1 hunks)
- src/components/container/BasicBadges/index.ts (1 hunks)
- src/components/container/BasicPointsForBadges/BasicPointsForBadges.css (1 hunks)
- src/components/container/BasicPointsForBadges/BasicPointsForBadges.stories.tsx (1 hunks)
- src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx (1 hunks)
- src/components/container/BasicPointsForBadges/index.ts (1 hunks)
- src/components/container/DailyDataGoal/DailyDataGoal.css (1 hunks)
- src/components/container/DailyDataGoal/DailyDataGoal.stories.tsx (1 hunks)
- src/components/container/DailyDataGoal/DailyDataGoal.tsx (1 hunks)
- src/components/container/DailyDataGoal/index.ts (1 hunks)
- src/components/container/index.ts (1 hunks)
- src/components/presentational/BasicBadge/BasicBadge.css (1 hunks)
- src/components/presentational/BasicBadge/BasicBadge.tsx (1 hunks)
- src/components/presentational/BasicBadge/index.ts (1 hunks)
- src/components/presentational/Divider/Divider.css (1 hunks)
- src/components/presentational/Divider/Divider.tsx (1 hunks)
- src/components/presentational/Divider/index.ts (1 hunks)
- src/components/presentational/Grid/Grid.css (1 hunks)
- src/components/presentational/Grid/Grid.stories.tsx (1 hunks)
- src/components/presentational/Grid/Grid.tsx (1 hunks)
- src/components/presentational/Grid/index.ts (1 hunks)
- src/components/presentational/ProgressBar/ProgressBar.css (2 hunks)
- src/components/presentational/ProgressBar/ProgressBar.tsx (2 hunks)
- src/components/presentational/index.ts (2 hunks)
- src/components/view/NewBadgeView/NewBadgeView.css (1 hunks)
- src/components/view/NewBadgeView/NewBadgeView.stories.tsx (1 hunks)
- src/components/view/NewBadgeView/NewBadgeView.tsx (1 hunks)
- src/components/view/NewBadgeView/index.ts (1 hunks)
- src/components/view/index.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/Activities.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/ConnectExternalAccountActivity.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/CustomActivity.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/DailyDataActivity.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/PointsAndBadges.ts (1 hunks)
- src/helpers/BasicPointsAndBadges/SurveyCompletionActivity.ts (1 hunks)
- src/helpers/colors.ts (1 hunks)
- src/helpers/daily-data-providers/fitbit-wear-minutes.ts (1 hunks)
- src/helpers/daily-data-providers/index.ts (1 hunks)
- src/helpers/daily-data-types.tsx (1 hunks)
- src/helpers/daily-data-types/fitbit.tsx (2 hunks)
- src/helpers/index.ts (1 hunks)
- src/helpers/query-daily-data.tsx (1 hunks)
- src/helpers/strings-en.ts (1 hunks)
- src/helpers/strings-es.ts (1 hunks)
Files not reviewed due to errors (4)
- src/helpers/colors.ts (no review received)
- src/helpers/BasicPointsAndBadges/DailyDataActivity.ts (no review received)
- src/helpers/BasicPointsAndBadges/Activities.ts (no review received)
- src/components/presentational/Grid/Grid.tsx (no review received)
Files skipped from review due to trivial changes (18)
- package.json
- src/components/container/BasicBadges/BasicBadges.css
- src/components/container/BasicBadges/index.ts
- src/components/container/BasicPointsForBadges/BasicPointsForBadges.css
- src/components/container/BasicPointsForBadges/index.ts
- src/components/container/DailyDataGoal/DailyDataGoal.css
- src/components/container/DailyDataGoal/index.ts
- src/components/presentational/BasicBadge/BasicBadge.css
- src/components/presentational/BasicBadge/index.ts
- src/components/presentational/Divider/Divider.css
- src/components/presentational/Divider/index.ts
- src/components/presentational/Grid/Grid.css
- src/components/presentational/Grid/index.ts
- src/components/view/NewBadgeView/NewBadgeView.css
- src/components/view/NewBadgeView/index.ts
- src/components/view/index.ts
- src/helpers/daily-data-providers/fitbit-wear-minutes.ts
- src/helpers/daily-data-providers/index.ts
Additional Context Used
Biome (181)
src/components/container/BasicBadges/BasicBadges.stories.tsx (1)
1-2: Some named imports are only used as types.
src/components/container/BasicBadges/BasicBadges.tsx (4)
21-21: Forbidden non-null assertion.
15-15: This let declares some variables that are only assigned once.
21-21: This let declares a variable that is only assigned once.
34-34: Avoid using the index of an array as key property in an element.
src/components/container/BasicPointsForBadges/BasicPointsForBadges.stories.tsx (2)
1-2: Some named imports are only used as types.
5-6: All these imports are only used as types.
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx (20)
37-37: Use === instead of ==.
== is only allowed when comparing againstnull
92-92: Use === instead of ==.
== is only allowed when comparing againstnull
92-92: Use === instead of ==.
== is only allowed when comparing againstnull
108-108: Use !== instead of !=.
!= is only allowed when comparing againstnull
108-108: Use !== instead of !=.
!= is only allowed when comparing againstnull
118-118: Unexpected any. Specify a different type.
124-124: Use === instead of ==.
== is only allowed when comparing againstnull
124-124: Use === instead of ==.
== is only allowed when comparing againstnull
4-5: Some named imports are only used as types.
7-8: Some named imports are only used as types.
9-10: Some named imports are only used as types.
26-26: This let declares some variables that are only assigned once.
27-27: This let declares some variables that are only assigned once.
28-28: This let declares a variable that is only assigned once.
35-35: This let declares a variable that is only assigned once.
57-57: This let declares a variable that is only assigned once.
65-65: This let declares a variable that is only assigned once.
66-66: This let declares a variable that is only assigned once.
70-70: This let declares a variable that is only assigned once.
76-76: This let declares a variable that is only assigned once.
src/components/container/DailyDataGoal/DailyDataGoal.stories.tsx (1)
4-5: Some named imports are only used as types.
src/components/container/DailyDataGoal/DailyDataGoal.tsx (16)
49-49: Template literals are preferred over string concatenation.
55-55: Forbidden non-null assertion.
1-1: The default import is only used as a type.
3-4: Some named imports are only used as types.
29-29: This let declares some variables that are only assigned once.
30-30: This let declares a variable that is only assigned once.
31-31: This let declares a variable that is only assigned once.
32-32: This let declares a variable that is only assigned once.
35-35: This let declares a variable that is only assigned once.
36-36: This let declares a variable that is only assigned once.
43-43: This let declares a variable that is only assigned once.
44-44: This let declares a variable that is only assigned once.
49-49: This let declares a variable that is only assigned once.
50-50: This let declares a variable that is only assigned once.
54-54: This let declares a variable that is only assigned once.
87-87: Avoid using unnecessary Fragment.
src/components/presentational/BasicBadge/BasicBadge.tsx (4)
1-1: Some named imports are only used as types.
1-2: Some named imports are only used as types.
16-16: This let declares a variable that is only assigned once.
17-17: This let declares a variable that is only assigned once.
src/components/presentational/Divider/Divider.tsx (1)
5-5: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
src/components/presentational/Grid/Grid.stories.tsx (5)
4-5: Some named imports are only used as types.
23-24: This let declares a variable that is only assigned once.
26-39: Missing key property for this element in iterable.
47-48: This let declares a variable that is only assigned once.
50-99: Missing key property for this element in iterable.
src/components/presentational/Grid/Grid.tsx (7)
35-35: Template literals are preferred over string concatenation.
44-49: This function expression can be turned into an arrow function.
25-25: This let declares a variable that is only assigned once.
33-33: This let declares a variable that is only assigned once.
45-45: This let declares a variable that is only assigned once.
46-46: This let declares a variable that is only assigned once.
47-47: This let declares a variable that is only assigned once.
src/components/presentational/ProgressBar/ProgressBar.tsx (7)
38-38: Template literals are preferred over string concatenation.
45-45: Template literals are preferred over string concatenation.
1-1: Some named imports are only used as types.
2-3: The default import and some named imports are only used as types.
29-29: This let declares a variable that is only assigned once.
30-30: This let declares a variable that is only assigned once.
45-45: Avoid using the index of an array as key property in an element.
src/components/view/NewBadgeView/NewBadgeView.stories.tsx (1)
1-2: Some named imports are only used as types.
src/components/view/NewBadgeView/NewBadgeView.tsx (2)
22-24: This function expression can be turned into an arrow function.
18-18: This let declares some variables that are only assigned once.
src/helpers/BasicPointsAndBadges/Activities.ts (5)
25-25: Prefer for...of instead of forEach.
25-25: The assignment should not be in an expression.
1-1: Some named imports are only used as types.
19-19: This let declares a variable that is only assigned once.
24-24: This let declares a variable that is only assigned once.
src/helpers/BasicPointsAndBadges/ConnectExternalAccountActivity.ts (6)
1-2: All these imports are only used as types.
14-14: This let declares a variable that is only assigned once.
15-15: This let declares a variable that is only assigned once.
16-16: This let declares a variable that is only assigned once.
18-18: This let declares a variable that is only assigned once.
19-19: This let declares a variable that is only assigned once.
src/helpers/BasicPointsAndBadges/CustomActivity.ts (6)
1-1: Some named imports are only used as types.
1-2: All these imports are only used as types.
10-10: This let declares a variable that is only assigned once.
11-11: This let declares a variable that is only assigned once.
12-12: This let declares a variable that is only assigned once.
13-13: This let declares a variable that is only assigned once.
src/helpers/BasicPointsAndBadges/DailyDataActivity.ts (7)
1-2: All these imports are only used as types.
17-17: This let declares a variable that is only assigned once.
23-23: This let declares a variable that is only assigned once.
24-24: This let declares a variable that is only assigned once.
25-25: This let declares a variable that is only assigned once.
27-27: This let declares a variable that is only assigned once.
28-28: This let declares a variable that is only assigned once.
src/helpers/BasicPointsAndBadges/PointsAndBadges.ts (10)
1-1: All these imports are only used as types.
1-2: All these imports are only used as types.
2-3: Some named imports are only used as types.
3-4: Some named imports are only used as types.
4-5: Some named imports are only used as types.
5-6: Some named imports are only used as types.
9-9: This let declares a variable that is only assigned once.
10-10: This let declares a variable that is only assigned once.
18-18: This let declares a variable that is only assigned once.
19-19: This let declares a variable that is only assigned once.
src/helpers/BasicPointsAndBadges/SurveyCompletionActivity.ts (9)
27-27: Use !== instead of !=.
!= is only allowed when comparing againstnull
32-32: Use !== instead of !=.
!= is only allowed when comparing againstnull
38-38: Use !== instead of !=.
!= is only allowed when comparing againstnull
1-1: All these imports are only used as types.
1-2: All these imports are only used as types.
17-17: This let declares a variable that is only assigned once.
19-21: Use let or const instead of var.
32-32: This let declares a variable that is only assigned once.
43-43: This let declares a variable that is only assigned once.
src/helpers/colors.ts (1)
15-51: Use let or const instead of var.
src/helpers/daily-data-types.tsx (2)
1-1: All these imports are only used as types.
1-2: All these imports are only used as types.
src/helpers/daily-data-types/fitbit.tsx (4)
182-182: Template literals are preferred over string concatenation.
203-206: Prefer for...of instead of forEach.
2-3: Some named imports are only used as types.
8-9: This let declares a variable that is only assigned once.
src/helpers/query-daily-data.tsx (20)
36-36: Template literals are preferred over string concatenation.
45-45: Template literals are preferred over string concatenation.
49-49: This var should be declared at the root of the enclosing function.
46-56: This function expression can be turned into an arrow function.
77-77: This var should be declared at the root of the enclosing function.
81-81: This var should be declared at the root of the enclosing function.
81-81: Template literals are preferred over string concatenation.
97-97: Forbidden non-null assertion.
100-102: Prefer for...of instead of forEach.
100-102: This function expression can be turned into an arrow function.
3-4: All these imports are only used as types.
14-14: This let declares a variable that is only assigned once.
35-35: Use let or const instead of var.
53-53: Reassigning a function parameter is confusing.
44-44: Use let or const instead of var.
47-47: Use let or const instead of var.
49-49: Use let or const instead of var.
84-84: Reassigning a function parameter is confusing.
60-60: Use let or const instead of var.
61-61: This let declares a variable that is only assigned once.
src/helpers/strings-en.ts (20)
2-2: The computed expression can be simplified without the use of a string literal.
3-3: The computed expression can be simplified without the use of a string literal.
4-4: The computed expression can be simplified without the use of a string literal.
5-5: The computed expression can be simplified without the use of a string literal.
6-6: The computed expression can be simplified without the use of a string literal.
7-7: The computed expression can be simplified without the use of a string literal.
8-8: The computed expression can be simplified without the use of a string literal.
9-9: The computed expression can be simplified without the use of a string literal.
10-10: The computed expression can be simplified without the use of a string literal.
11-11: The computed expression can be simplified without the use of a string literal.
12-12: The computed expression can be simplified without the use of a string literal.
13-13: The computed expression can be simplified without the use of a string literal.
27-27: The computed expression can be simplified without the use of a string literal.
28-28: The computed expression can be simplified without the use of a string literal.
39-39: The computed expression can be simplified without the use of a string literal.
42-42: The computed expression can be simplified without the use of a string literal.
45-45: The computed expression can be simplified without the use of a string literal.
47-47: The computed expression can be simplified without the use of a string literal.
48-48: The computed expression can be simplified without the use of a string literal.
49-49: The computed expression can be simplified without the use of a string literal.
src/helpers/strings-es.ts (20)
2-2: The computed expression can be simplified without the use of a string literal.
3-3: The computed expression can be simplified without the use of a string literal.
4-4: The computed expression can be simplified without the use of a string literal.
5-5: The computed expression can be simplified without the use of a string literal.
6-6: The computed expression can be simplified without the use of a string literal.
7-7: The computed expression can be simplified without the use of a string literal.
8-8: The computed expression can be simplified without the use of a string literal.
9-9: The computed expression can be simplified without the use of a string literal.
10-10: The computed expression can be simplified without the use of a string literal.
11-11: The computed expression can be simplified without the use of a string literal.
12-12: The computed expression can be simplified without the use of a string literal.
13-13: The computed expression can be simplified without the use of a string literal.
27-27: The computed expression can be simplified without the use of a string literal.
28-28: The computed expression can be simplified without the use of a string literal.
40-40: The computed expression can be simplified without the use of a string literal.
42-43: The computed expression can be simplified without the use of a string literal.
46-46: The computed expression can be simplified without the use of a string literal.
48-48: The computed expression can be simplified without the use of a string literal.
48-49: The computed expression can be simplified without the use of a string literal.
50-50: The computed expression can be simplified without the use of a string literal.
Additional comments not posted (20)
src/helpers/index.ts (1)
12-12: The export ofPointsAndBadgescorrectly aligns with the new functionality introduced in this PR.src/components/presentational/ProgressBar/ProgressBar.css (1)
11-11: The CSS changes enhance the visual presentation of the progress bar and align with the PR's objectives to improve user interface elements.Also applies to: 16-16, 22-23, 29-30, 32-32, 36-36, 40-40, 52-52
src/components/presentational/BasicBadge/BasicBadge.tsx (1)
15-26: The component structure and logic appear well-implemented and clear.src/components/presentational/ProgressBar/ProgressBar.tsx (2)
39-39: Consider using template literals for string concatenation to improve readability and maintainability.- <div className={classes.join(" ")} ref={props.innerRef}> + <div className={`${classes.join(" ")}`} ref={props.innerRef}>Likely invalid or redundant comment.
5-5: EnsureShinyOverlayis properly integrated and tested.src/components/container/BasicBadges/BasicBadges.tsx (1)
5-5: EnsureMyDataHelpsis properly integrated and tested.src/components/container/DailyDataGoal/DailyDataGoal.stories.tsx (1)
45-45: Ensure the correct daily data type is used for the Wear Time Goal.src/components/container/index.ts (1)
3-4: Export statements for new components are correctly added.Also applies to: 11-11
src/components/presentational/index.ts (1)
3-3: Export statements for new presentational components are correctly added.Also applies to: 13-13, 16-16
src/helpers/daily-data-types.tsx (1)
40-40: The addition ofFitbitWearMinutesto the enum is correctly implemented.src/components/container/BasicPointsForBadges/BasicPointsForBadges.stories.tsx (4)
27-43: TheDefaultstory configuration is well-defined and correctly sets up a default scenario for theBasicPointsForBadgescomponent.
46-63: TheShowTotalPointsstory configuration effectively demonstrates the component's ability to display total points.
66-140: TheWithGoalsAndBadgesstory configuration is comprehensive and effectively demonstrates the component's integration withDailyDataGoalandBasicBadges.
144-180: TheLivestory configuration is robust, representing a realistic live scenario with various activities and a custom field.src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx (3)
30-32: ThesumActivityPointsfunction correctly implements the logic to calculate the total points awarded across activities.
34-50: ThepreviewParticipantInfofunction effectively creates a mockParticipantInfoobject for preview purposes, aiding in component testing.
53-61: ThepreviewAwardPointsAndBadgesfunction provides a necessary simulation for awarding points and badges in preview scenarios.src/helpers/daily-data-types/fitbit.tsx (1)
195-201: The definition forFitbitWearMinutesis well-implemented, following the established pattern for defining daily data types.src/helpers/strings-en.ts (1)
368-374: The new string definitions related to the points and badges system are correctly formatted and relevant to the new features.src/helpers/strings-es.ts (1)
368-369: The new localization strings for the points and badges system are correctly added and formatted.
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
|
This should be ready for review now after a handful of iterations / tweaks |
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (3)
src/helpers/colors.ts (1)
16-49: Consider documenting the purpose of thecolorAssortmentarray and why these specific colors are chosen.Adding comments explaining the choice of colors can enhance the maintainability of the code, especially for new team members or external contributors.
src/helpers/query-daily-data.tsx (2)
Line range hint
47-57: Convert function expression to arrow function for consistency.- return dailyDataType.dataProvider(startDate, endDate).then(function (data) { + return dailyDataType.dataProvider(startDate, endDate).then((data) => { var result: DailyDataQueryResult = {}; while (startDate < endDate) { var dayKey = getDayKey(startDate); if (data[dayKey]) { result[dayKey] = data[dayKey]; } startDate = add(startDate, { days: 1 }); } return result; });
Line range hint
86-88: Convert function expression to arrow function for consistency.- allTypeDefinitions.forEach(function (typeDefinition) { + allTypeDefinitions.forEach((typeDefinition) => { registerDailyDataTypeDefinition(typeDefinition); });
src/helpers/BasicPointsAndBadges/ConnectExternalAccountActivity.ts
Outdated
Show resolved
Hide resolved
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
| </> | ||
| </BasicBadge> | ||
| </div> | ||
| <Title order={2} style={{ margin: "16px", marginBottom: 0 }}>{props.title || language("new-badge-title")}</Title> |
| import "./BasicBadges.css" | ||
| import MyDataHelps from "@careevolution/mydatahelps-js"; | ||
| import { parsePointsAndBadgesState } from "../../../helpers/BasicPointsAndBadges/Activities"; | ||
| import { FontAwesomeSvgIcon } from "react-fontawesome-svg-icon"; |
| <DailyDataGoal {...args} /> | ||
| </Layout>; | ||
|
|
||
| export const SleepGoal = { |
There was a problem hiding this comment.
Are these stories meant to demonstrate a difference in presentation based on dailyDataType or how close someone was/not to their goal? I don't see a difference in the test data generated.
These look more like Goal Achieved, Goal Progress, and Goal No Progress.
There was a problem hiding this comment.
Basically just exercising a real-world example. Sleep in this case someone gets credit for even if they just had 1 minute of sleep recorded; whereas wear time only counts if you hit 10 hours etc.
There was a problem hiding this comment.
Code looks good. Just some general questions.
Tested against GarminActiveMinutes data and custom field content (needed to update below with the GarminActiveMinutes)
{"badges":[1000,2000,3000,4000,5000,6000],"activityStates":{"FitbitSleepData":{"pointsAwarded":3800,"daysAwarded":["2024-07-08","2024-07-09","2024-07-10","2024-07-11","2024-07-12","2024-07-14","2024-07-15","2024-07-16","2024-07-17","2024-07-18","2024-07-19","2024-07-20","2024-07-21","2024-07-22","2024-07-23","2024-07-24","2024-07-25","2024-07-26","2024-07-27"]},"FitbitWearTime":{"pointsAwarded":1900,"daysAwarded":["2024-07-09","2024-07-08","2024-07-10","2024-07-11","2024-07-12","2024-07-13","2024-07-14","2024-07-15","2024-07-16","2024-07-17","2024-07-18","2024-07-19","2024-07-20","2024-07-21","2024-07-22","2024-07-23","2024-07-24","2024-07-25","2024-07-26"]},"FitbitOrder":{"countCompleted":1,"lastQueryDate":"2024-07-09T23:16:37.77Z","pointsAwarded":800},"ConnectEhr":{"pointsAwarded":0}}}
| } | ||
| } | ||
|
|
||
| async function previewAwardPointsAndBadges( |
There was a problem hiding this comment.
Move preview data out into .previewdata file
|
|
||
| if (currentState.badges.length < updatedState.badges.length) { | ||
| await new Promise(resolve => setTimeout(resolve, 3000)); | ||
| MyDataHelps.openApplication(props.awardBadgesViewUrl, { modal: true }); |
There was a problem hiding this comment.
Confirming the user is shown a modal here?
I'm interested in learning the use case for this
There was a problem hiding this comment.
Yep, there's a popup that shows them their new badge
| title?: string; | ||
| text?: string; | ||
| colorScheme?: 'auto' | 'light' | 'dark'; | ||
| primaryColor?: string; |
There was a problem hiding this comment.
Should this be ColorDefinition?
| @@ -0,0 +1,29 @@ | |||
| import type { ReactElement } from "react"; | |||
There was a problem hiding this comment.
Was no stories intentional?
| @@ -0,0 +1,6 @@ | |||
| import React from "react"; | |||
| import "./Divider.css" | |||
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (9)
src/helpers/BasicPointsAndBadges/ConnectExternalAccountActivity.ts (2)
1-2: Useimport typefor type-only imports.To clarify their usage and potentially optimize bundling, use
import typefor imports that are only used for type annotations.- import type { BasicPointsForBadgesActivity, BasicPointsForBadgesActivityState } from "./Activities"; + import { BasicPointsForBadgesActivity, BasicPointsForBadgesActivityState } from "./Activities";
13-22: Useconstfor variables that are not reassigned.Change
lettoconstfor variables that are not reassigned to follow best practices and clarify intent.- let connectedAccounts = await MyDataHelps.getExternalAccounts(); - let allProviders = connectedAccounts.filter(account => activity.providerCategories.includes(account.provider.category)).map(t => t.id); - let newProviders = allProviders.filter(provider => !activityState.providersConnected?.includes(provider)); - let newPoints = newProviders.length * activity.points; - let newActivityState = { pointsAwarded: activityState.pointsAwarded + newPoints, providersConnected: allProviders }; + const connectedAccounts = await MyDataHelps.getExternalAccounts(); + const allProviders = connectedAccounts.filter(account => activity.providerCategories.includes(account.provider.category)).map(t => t.id); + const newProviders = allProviders.filter(provider => !activityState.providersConnected?.includes(provider)); + const newPoints = newProviders.length * activity.points; + const newActivityState = { pointsAwarded: activityState.pointsAwarded + newPoints, providersConnected: allProviders };src/helpers/BasicPointsAndBadges/PointsAndBadges.ts (5)
1-1: Useimport typefor type-only imports.To clarify their usage and potentially optimize bundling, use
import typefor imports that are only used for type annotations.- import { ParticipantInfo } from "@careevolution/mydatahelps-js"; + import type { ParticipantInfo } from "@careevolution/mydatahelps-js";
9-10: Useconstfor variables that are not reassigned.Change
lettoconstfor variables that are not reassigned to follow best practices and clarify intent.- let awardPointsPromises = activities.map((activity) => awardPointsForActivity(activity, state.activityStates[activity.key], participantInfo)); + const awardPointsPromises = activities.map((activity) => awardPointsForActivity(activity, state.activityStates[activity.key], participantInfo));
18-19: Useconstfor variables that are not reassigned.Change
lettoconstfor variables that are not reassigned to follow best practices and clarify intent.- let newPointTotal = activities.reduce((sum, activity) => sum + updatedActivityStates[activity.key].pointsAwarded, 0); + const newPointTotal = activities.reduce((sum, activity) => sum + updatedActivityStates[activity.key].pointsAwarded, 0);
19-20: Useconstfor variables that are not reassigned.Change
lettoconstfor variables that are not reassigned to follow best practices and clarify intent.- let lastBadge = state.badges.length ? Math.max(...state.badges) : 0; + const lastBadge = state.badges.length ? Math.max(...state.badges) : 0;
20-21: Useletinstead ofconstfor reassigned variables.Change
consttoletfor variables that are reassigned to follow best practices and clarify intent.- const nextBadge = lastBadge + pointsPerBadge; + let nextBadge = lastBadge + pointsPerBadge;src/components/view/NewBadgeView/NewBadgeView.tsx (1)
1-17: Ensure consistent import style.Consider using named imports for consistency and readability.
- import MyDataHelps from "@careevolution/mydatahelps-js"; + import { MyDataHelps } from "@careevolution/mydatahelps-js";src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx (1)
1-22: Ensure consistent import style.Consider using named imports for consistency and readability.
- import MyDataHelps, { ParticipantInfo } from "@careevolution/mydatahelps-js"; + import { MyDataHelps, ParticipantInfo } from "@careevolution/mydatahelps-js";
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
src/components/container/BasicPointsForBadges/BasicPointsForBadges.tsx
Outdated
Show resolved
Hide resolved
| export function previewParticipantInfo(activities: BasicPointsForBadgesActivity[], pointsPerBadge: number, customField: string): ParticipantInfo { | ||
| let previewActivityStates: { [key: string]: BasicPointsForBadgesActivityState } = {}; | ||
| activities.forEach((activity, index) => { | ||
| previewActivityStates[activity.key] = { pointsAwarded: index == 0 ? 300 + (2 * pointsPerBadge) : 0 }; |
There was a problem hiding this comment.
Use strict equality.
Replace index == 0 with index === 0 for strict equality.
- previewActivityStates[activity.key] = { pointsAwarded: index == 0 ? 300 + (2 * pointsPerBadge) : 0 };
+ previewActivityStates[activity.key] = { pointsAwarded: index === 0 ? 300 + (2 * pointsPerBadge) : 0 };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.
| previewActivityStates[activity.key] = { pointsAwarded: index == 0 ? 300 + (2 * pointsPerBadge) : 0 }; | |
| previewActivityStates[activity.key] = { pointsAwarded: index === 0 ? 300 + (2 * pointsPerBadge) : 0 }; |
| //@ts-ignore | ||
| demographics: {}, |
There was a problem hiding this comment.
Avoid using @ts-ignore.
Using @ts-ignore can hide potential issues. If possible, provide a proper type definition for demographics.
- //@ts-ignore
- demographics: {},
+ demographics: {} as DemographicsType, // Replace `DemographicsType` with the appropriate typeCommittable suggestion was skipped due to low confidence.
src/components/container/BasicPointsForBadges/BasicPointsForBadges.previewData.ts
Show resolved
Hide resolved
| {badgeCount && Array.from({ length: displayedCount }).map((_, index) => | ||
| <BasicBadge key={`badge-${index}`} backgroundColor={getColorFromAssortment(badgeCount! - index - 1)} /> |
There was a problem hiding this comment.
Avoid using the index as a key in React lists.
Using the index as a key can lead to potential issues with component state and re-rendering.
- {badgeCount && Array.from({ length: displayedCount }).map((_, index) =>
- <BasicBadge key={`badge-${index}`} backgroundColor={getColorFromAssortment(badgeCount! - index - 1)} />
+ {badgeCount && Array.from({ length: displayedCount }).map((_, index) => {
+ const key = `badge-${badgeCount! - index - 1}`;
+ return <BasicBadge key={key} backgroundColor={getColorFromAssortment(badgeCount! - index - 1)} />
+ })}Committable suggestion was skipped due to low confidence.
Overview
This introduces a fairly basic set of components for a points and badges system. The overall design here is already IRB-approved; while this definitely has some limitations (notably mostly client-side calculation), I think it will work nicely as a drop-in component for studies who just want to add a little "gamification" to their project.
There's a handful of components here:
There's also a supporting cast of presentational components:
Points and Badges Calculation
The calculations are all performed client side at the moment (with the option of also getting points from a custom field); which of course has a bunch of downsides (can't send notifications of goals being completed passively, not advisable to award gift cards based on this) but for many use cases where this is a bonus engagement thing will probably work fine.
The BasicPointsForBadges allows specifying a certain set of activities, each of which have a point total associated with them. The current supported activities are:
The data underlying this is all stored in a (writeable) JSON custom field. Each activity has a corresponding "state" in the JSON, which stores the points it has awarded so far and any data it needs to ensure it does not award additional points for the same activity (e.g. the previously connected providers). The state also stores the current badges as an array of point totals for when those badges were reached.
This design allows for some flexibility for changing point totals moving forward. For instance:
Of course, since this is in a custom field, we can clear out the custom field at any time if we want to "reset" a user's points and badges for them; or also edit the custom field JSON.
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:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
BasicBadgesandBasicPointsForBadgescomponents for displaying badges and points towards badges.DailyDataGoalcomponent to display daily data goals.NewBadgeViewcomponent to show a detailed badge view with animations.Style
NewBadgeViewandDailyDataGoalcomponents.Localization
Bug Fixes