-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Mobile Updates & Repo clean up #131
Conversation
oreHGA
commented
Jul 23, 2023
•
edited
Loading
edited
- New insights page with filter for "Weekly" & "Monthly"
- Support for yes/no, number, customOptions & text responses
- Remove /analysis directory as that should be in a different repo
- Fix to the analysis charts (allowing for displaying powerByBand over time)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, I like how it's coming together. I know it's not done yet but I just thought I'd add my review now that I have some time
fusionMobile/src/pages/home.tsx
Outdated
React.useEffect(() => { | ||
if (activeChartPrompt) { | ||
console.log("activeChartPrompt", activeChartPrompt); | ||
(async () => { | ||
const res = await promptService.getPromptResponses( | ||
activeChartPrompt.uuid | ||
); | ||
|
||
// sort events | ||
res.sort((a, b) => { | ||
return a.triggerTimestamp - b.triggerTimestamp; | ||
}); | ||
|
||
setActiveChartPromptResponses(res); | ||
})(); | ||
} | ||
}, [activeChartPrompt]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be extracted into a useQuery hook that handles the fetching, the good thing about this is we can do a check to see if the uuid of the active prompt is present and then enable/disable our query like this
React.useEffect(() => { | |
if (activeChartPrompt) { | |
console.log("activeChartPrompt", activeChartPrompt); | |
(async () => { | |
const res = await promptService.getPromptResponses( | |
activeChartPrompt.uuid | |
); | |
// sort events | |
res.sort((a, b) => { | |
return a.triggerTimestamp - b.triggerTimestamp; | |
}); | |
setActiveChartPromptResponses(res); | |
})(); | |
} | |
}, [activeChartPrompt]); | |
const [activeChartPrompt, setActiveChartPrompt] = React.useState< | |
Prompt | undefined | |
>(); | |
const activeChartPromptId = activeChartPrompt.uuid | |
const queryInfo = useQuery({ | |
queryKey: ["promptResponses", activeChartPromptId], | |
queryFn: () => promptService.getPromptResponses(activeChartPromptId), | |
// The query will not execute until the activeChartPromptId exists | |
enabled: Boolean(activeChartPromptId), | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #140
(async () => { | ||
const res = await promptService.getPromptResponses(prompt.uuid); | ||
|
||
if (!res) { | ||
setChartData([]); | ||
} else { | ||
// sort events | ||
res.sort((a, b) => { | ||
return a.triggerTimestamp - b.triggerTimestamp; | ||
}); | ||
|
||
// filter by startDate | ||
const filteredData = res | ||
.filter((response) => { | ||
const responseTimestamp = updateTimestampToMs( | ||
response.responseTimestamp | ||
); | ||
|
||
return ( | ||
dayjs(responseTimestamp).isAfter(startDate) && | ||
dayjs(responseTimestamp).isBefore(startDate.endOf(timePeriod)) | ||
); | ||
}) | ||
.map((d) => { | ||
return [updateTimestampToMs(d.responseTimestamp), d.value]; | ||
}); | ||
|
||
// TODO: make this better pls | ||
if (prompt.responseType === "number" && filteredData.length > 0) { | ||
// group the responses by startTimestamp of the day | ||
const dailyAverage: any = []; // [startTimestamp, average] | ||
// just make it an object | ||
const dailyAverageObj: any = {}; | ||
|
||
filteredData.forEach((d) => { | ||
const day = dayjs(d[0]).startOf("day").valueOf(); | ||
const value = convertValueToNumber(d[1]); | ||
|
||
console.log(day, value); | ||
if (dailyAverageObj[day]) { | ||
dailyAverageObj[day].push(value); | ||
} else { | ||
dailyAverageObj[day] = [value]; | ||
} | ||
}); | ||
|
||
console.log(dailyAverageObj); | ||
|
||
// calculate the average | ||
Object.keys(dailyAverageObj).forEach((key) => { | ||
const average = | ||
dailyAverageObj[key].reduce((a: number, b: number) => a + b, 0) / | ||
dailyAverageObj[key].length; | ||
dailyAverage.push([Number(key), average]); | ||
}); | ||
|
||
setChartData(dailyAverage); | ||
return; | ||
} | ||
|
||
setChartData(filteredData); | ||
} | ||
})(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another one we should definitely aim to convert to a reusable custom hook that uses useQuery
under the hood
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #140
75fce19
to
9106555
Compare
1d9beb5
to
6970339
Compare
6970339
to
8bcf08a
Compare
Merging this as is now, i've created issue to track the fix for #140 |