Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
API/public #24
base: HealthChart/main
Are you sure you want to change the base?
API/public #24
Changes from all commits
b68d3fd
f8c950e
124045d
cbbc335
4f5ff38
5713a64
5bf0d21
0948f21
9030f5c
4452419
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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'm not sure if this works in the end, as this will not be used for view re-render by SwiftUI. Not sure if it is needed though. Just a little note.
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.
Hi @Supereg! Would you mind explaining a little more about this? When I tested the UI with this setup, I saw that when I change the date range in the lowest view in the hierarchy, this binding propagated that change up to the top level as expected, and vice versa when I change the range at the highest level. That is, the views seem to re-render correctly when I change the state? Let me know if I'm misunderstanding what you mean!
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.
What was the reason to have the concept of the
DataProvider
be part of the public interface here? Would it make sense to just to just pass a collection of (HK) samples to the chart to be as reusable as possible. For example, if I just have a few recordings form my Bluetooth device that I turned into HKQuantitySamples, I can just pass the collection to the Chart.I think the concept of the
HealthKitDataProvider
API is great, especially to make it easier to query samples from the HealthKitStore. But can these two APIs be separate? Or have a chart view that works with a DataProvider be a separate component that sits on top of a simple chart implementation that just takes a collection of samples?Let me know what your thoughts are here 👍
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 think a naming like
healthChartStyle(_:)
would be more unique and fit great with existing SwiftUI APIs (likebuttonStyle(_:)
orlabelStye(_:)
, ...).Check warning on line 15 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test Swift Package / Test using xcodebuild or run fastlane
Check warning on line 15 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test Swift Package / Test using xcodebuild or run fastlane
Check warning on line 15 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test UI Tests / Test using xcodebuild or run fastlane
Check warning on line 15 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test UI Tests / Test using xcodebuild or run fastlane
Check failure on line 31 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test UI Tests / Test using xcodebuild or run fastlane
Check warning on line 108 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test Swift Package / Test using xcodebuild or run fastlane
Check warning on line 108 in Sources/SpeziHealthCharts/MetricChart/DataProvider/HealthKitDataProvider/HealthKitDataProvider.swift
GitHub Actions / Build and Test UI Tests / Test using xcodebuild or run fastlane