-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add Sleep Data Chart #503
Add Sleep Data Chart #503
Conversation
Pull Request Test Coverage Report for Build 7263537534
💛 - Coveralls |
{component.type === 'SleepChart' && ( | ||
<SleepChart | ||
{...component} | ||
title="Sleep Analysis" |
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.
Should use i18n
?
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.
Oh good catch. That shouldn't be there. The chart title should come from the app config. I'll update this.
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.
I'll fix this in a followup pull request.
.range([0, common.plotAreaWidth]) | ||
.domain([new Date(start ?? 0), new Date(end ?? 0)]); | ||
|
||
setChartData({ sleepData: newSleepData, xDomain: newXDomain, dateRange }); |
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.
Like we've talked about, whether in this hook or elsewhere, I foresee the need to filter newSleepData
down to handle multiple sleep records per evening (e.g. one from Oura, one from Garmin -> both synced in from Apple Health). Unless that logic is already in place elsewhere, just keep it in mind (as perhaps a separate task to eventually get to). Excited to try this out with my own data, which would serve as an example of that scenario.
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.
I kind of forgot about that discussion. I think it will likely have some display issues with the current solution. I look into it and follow up with a fix.
3: t('sleep-analysis-type-rem', 'REM'), | ||
2: t('sleep-analysis-type-light', 'Light'), | ||
1: t('sleep-analysis-type-deep', 'Deep'), | ||
}[value] ?? ''); |
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.
🔥 so cool to see these normalized component codings come together!
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.
🔥
🎉 This PR is included in version 11.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
SleepChart
Screenshots