Charts
Charts copied to clipboard
danielgindi
[WIP] Release/5.0.0
Work to release a 5.0 version of Charts. The main goal of the release is to rename the library to DGCharts.
tried ChartsDemo-iOS, not build when BUILD_LIBRARY_FOR_DISTRIBUTION is on I have filed a post https://forums.swift.org/t/bug-report-unable-to-build-demo-project-if-imported-swift-framework-turned-on-build-library-for-distribution/63904
radar: https://feedbackassistant.apple.com/feedback/12078082
btw, have you tried running the tests, should we update the test images using new Xcode? Previously I ran into failures because the UIKit update something and the images had like 0.1% or 1% diff, mostly like the font is a little bit bolder or slightly position shift.
tried ChartsDemo-iOS, not build when BUILD_LIBRARY_FOR_DISTRIBUTION is on I have filed a post https://forums.swift.org/t/bug-report-unable-to-build-demo-project-if-imported-swift-framework-turned-on-build-library-for-distribution/63904
Yeah this is the issue I ran into as well.
btw, have you tried running the tests, should we update the test images using new Xcode? Previously I ran into failures because the UIKit update something and the images had like 0.1% or 1% diff, mostly like the font is a little bit bolder or slightly position shift.
Hmm yeah I don’t know. The test run fine on CI but it might be a good idea. Could you add an issue to the 5.0 milestone so we don’t forget.
btw, have you tried running the tests, should we update the test images using new Xcode? Previously I ran into failures because the UIKit update something and the images had like 0.1% or 1% diff, mostly like the font is a little bit bolder or slightly position shift.
Hmm yeah I don’t know. The test run fine on CI but it might be a good idea. Could you add an issue to the 5.0 milestone so we don’t forget.
https://github.com/danielgindi/Charts/issues/5033.
I do see Swift / iOS (OS=16.0,name=iPhone 14 Pro) (pull_request)
Successful in 8m ran 66 tests and passed.
On my mac it is
Executed 66 tests, with 52 failures (0 unexpected) in 12.944 (13.016) seconds
Program ended with exit code: 1,
Xcode Version 14.2 (14C18), 14 Pro Simulator 16.2.
Compared two diffs, mostly the label diff, the issue is the label I generated is grey, while the test image is black It's weird I thouched nothing and the color has huge difference. I didn't specify grey.
On my mac it is
Executed 66 tests, with 52 failures (0 unexpected) in 12.944 (13.016) seconds Program ended with exit code: 1,Xcode Version 14.2 (14C18), 14 Pro Simulator 16.2.
Compared two diffs, mostly the label diff, the issue is the label I generated is grey, while the test image is black It's weird I thouched nothing and the color has huge difference. I didn't specify grey.
Toggle Appearance option for iOS simulator changes test results:
Dark:
Light:
Xcode 14.2 (14C18), 14 Pro Simulator 16.2
tried ChartsDemo-iOS, not build when BUILD_LIBRARY_FOR_DISTRIBUTION is on I have filed a post https://forums.swift.org/t/bug-report-unable-to-build-demo-project-if-imported-swift-framework-turned-on-build-library-for-distribution/63904
radar: https://feedbackassistant.apple.com/feedback/12078082
I didn't get any reply from Swift forum nor radar. Not sure what we should go for, either wait or make a release but pointing out the Objc'demo is broken
On my mac it is
Executed 66 tests, with 52 failures (0 unexpected) in 12.944 (13.016) seconds Program ended with exit code: 1,Xcode Version 14.2 (14C18), 14 Pro Simulator 16.2. Compared two diffs, mostly the label diff, the issue is the label I generated is grey, while the test image is black It's weird I thouched nothing and the color has huge difference. I didn't specify grey.
Toggle Appearance option for iOS simulator changes test results: Dark:
it shouldn't bother? or the dark mode changed the label color or background color? Normally we don't care dark or not. Just make sure the chart positions itself correctly.
On my mac it is
Executed 66 tests, with 52 failures (0 unexpected) in 12.944 (13.016) seconds Program ended with exit code: 1,Xcode Version 14.2 (14C18), 14 Pro Simulator 16.2. Compared two diffs, mostly the label diff, the issue is the label I generated is grey, while the test image is black It's weird I thouched nothing and the color has huge difference. I didn't specify grey.
Toggle Appearance option for iOS simulator changes test results: Dark:
it shouldn't bother? or the dark mode changed the label color or background color? Normally we don't care dark or not. Just make sure the chart positions itself correctly.
In dark mode where only BarChartTests failed, simulator labels' colors were white, but chart positions remained the same:
Snapshot images for Logical width 393 and Logical height 852(e.g. iPhone 14 Pro) have different label colors for different chart types:
If I update BarChartTests for that logical size with white label snapshots then all tests pass:
Any update here? Who is the right person to make the call as to whether or not these test cases actually matter?
Any update here? Who is the right person to make the call as to whether or not these test cases actually matter?
Hey they don’t actually matter to release. I think we can just force light mode in the tests. The real problem is https://github.com/danielgindi/Charts/pull/5028#issuecomment-1480916120
It is a blocker. If you have ideas I’m open to them :p
tried ChartsDemo-iOS, not build when BUILD_LIBRARY_FOR_DISTRIBUTION is on I have filed a post https://forums.swift.org/t/bug-report-unable-to-build-demo-project-if-imported-swift-framework-turned-on-build-library-for-distribution/63904
radar: https://feedbackassistant.apple.com/feedback/12078082
Hey guys, bumping the minimum iOS version of the ChartsDemo-iOS target to iOS 13 resolves the issue. I've actually created a fresh Objective-C project, added the binary framework, created the Swift files to use the marker subclasses and as soon as I lowered the minimum deployment target to < 13.0 the build error appear. Hope that helps and we can get a release soon.
Also, can we add #5041 and #5043 to this release? That would be awesome!
@pmairoldi do you think it's reasonable to bump the minimum version as @FelixHerrmann mentioned?
@pmairoldi is there any way I could assist in this. Is a new PR required just to bump the min version and test?