ChartsOrg / Charts

Beautiful charts for iOS/tvOS/OSX! The Apple side of the crossplatform MPAndroidChart.
Apache License 2.0
27.51k stars 5.99k forks source link

[WIP] Release/5.0.0 #5028

Closed pmairoldi closed 1 year ago

pmairoldi commented 1 year ago

Work to release a 5.0 version of Charts. The main goal of the release is to rename the library to DGCharts.

liuxuan30 commented 1 year ago

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

liuxuan30 commented 1 year ago

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.

pmairoldi commented 1 year ago

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.

pmairoldi commented 1 year ago

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.

liuxuan30 commented 1 year ago

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.

image image

rineek commented 1 year ago

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.

image image

Toggle Appearance option for iOS simulator changes test results: Dark: Screenshot 2023-03-29 at 19 08 23 Light: Screenshot 2023-03-29 at 19 09 43 Xcode 14.2 (14C18), 14 Pro Simulator 16.2

liuxuan30 commented 1 year ago

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

liuxuan30 commented 1 year ago

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. image image

Toggle Appearance option for iOS simulator changes test results: Dark: Screenshot 2023-03-29 at 19 08 23 Light: Screenshot 2023-03-29 at 19 09 43 Xcode 14.2 (14C18), 14 Pro Simulator 16.2

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.

rineek commented 1 year ago

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. image image

Toggle Appearance option for iOS simulator changes test results: Dark: Screenshot 2023-03-29 at 19 08 23 Light: Screenshot 2023-03-29 at 19 09 43 Xcode 14.2 (14C18), 14 Pro Simulator 16.2

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: Screenshot 2023-04-12 at 15 37 51 Snapshot images for Logical width 393 and Logical height 852(e.g. iPhone 14 Pro) have different label colors for different chart types: Screenshot 2023-04-12 at 15 48 56 Screenshot 2023-04-12 at 15 49 04 If I update BarChartTests for that logical size with white label snapshots then all tests pass: Screenshot 2023-04-12 at 16 12 48

liuxuan30 commented 1 year ago

@rineek usually we don't care too much about dark mode tests.

waterskier2007 commented 1 year ago

Any update here? Who is the right person to make the call as to whether or not these test cases actually matter?

pmairoldi commented 1 year ago

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

FelixHerrmann commented 1 year ago

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.

FelixHerrmann commented 1 year ago

Also, can we add #5041 and #5043 to this release? That would be awesome!

pmairoldi commented 1 year ago

@FelixHerrmann thanks. Will look at all that maybe this weekend.

waterskier2007 commented 1 year ago

@pmairoldi do you think it's reasonable to bump the minimum version as @FelixHerrmann mentioned?

pmairoldi commented 1 year ago

I haven’t tried yet.

waterskier2007 commented 1 year ago

@pmairoldi is there any way I could assist in this. Is a new PR required just to bump the min version and test?

waterskier2007 commented 1 year ago

https://github.com/danielgindi/Charts/pull/5062

waterskier2007 commented 1 year ago

@pmairoldi any update on when we might be able to get a release?

liuxuan30 commented 1 year ago

so, seems good to go? just try to compare the light mode tests?

pmairoldi commented 1 year ago

There are a couple points left in milestone 5.0.0

liuxuan30 commented 1 year ago

eh. I will see what I could take on tomorrow

pmairoldi commented 1 year ago

5.0.0 has been released.

liuxuan30 commented 1 year ago

cheers, maybe get a beer from the funds!