Support other bundle than main MarkerView.viewFromXib()#3215
Support other bundle than main MarkerView.viewFromXib()#3215liuxuan30 merged 4 commits intoChartsOrg:masterfrom charlymr:master
Conversation
jjatie
left a comment
There was a problem hiding this comment.
Looks good. Just updated based on review and we'll merge for 3.1
|
|
||
| @objc | ||
| open class func viewFromXib() -> MarkerView? | ||
| open class func viewFromXib() -> MarkerView? { |
There was a problem hiding this comment.
please put the { on a new line.
| } | ||
|
|
||
| @objc | ||
| open class func viewFromXib(in bundle: Bundle) -> MarkerView? |
There was a problem hiding this comment.
This can be one method: open class func viewFromXib(in bundle: Bundle = .main) -> MarkerView? instead of two.
There was a problem hiding this comment.
Yeah that's what I was thinking in the first place, however if someone is overriding that method it will break his code. (@objc) is declared as well here. So even more important!
What do you think?
There was a problem hiding this comment.
The main reason to override this method is to change the bundle. If it breaks their code, this is a very minor change and we will have a few possible minor code breaking changes in 3.1.
Let's make this one method.
There was a problem hiding this comment.
And keep the { on a new line.
There was a problem hiding this comment.
All done. Thanks. Great job by the way :)
|
Please update the macOS demo for the new method signature. |
Codecov Report
@@ Coverage Diff @@
## master #3215 +/- ##
=======================================
Coverage 22.89% 22.89%
=======================================
Files 116 116
Lines 15609 15609
Branches 272 272
=======================================
Hits 3573 3573
Misses 12000 12000
Partials 36 36
Continue to review full report at Codecov.
|
|
@liuxuan30 I approve, I'll give you a chance to review. I know you're busy. |
|
|
||
| @objc | ||
| open class func viewFromXib() -> MarkerView? | ||
| open class func viewFromXib(in bundle: Bundle = .main) -> MarkerView? |
There was a problem hiding this comment.
I don't realize @objc can be used for functions with default parameters.. :)
|
Seems fine. Though busy, I can afford some time every day in the morning. |
* 'master' of https://github.com/danielgindi/Charts: Restored old performance (ChartsOrg#3216) Support other bundle than main MarkerView.viewFromXib() (ChartsOrg#3215) For ChartsOrg#2840. add dataIndex parameter in `highlightValue()` calls (ChartsOrg#2852) Balloon Marker indicates position of data (ChartsOrg#3181) bump Info.plist version BubbleChart uses correct colour for index now. # Conflicts: # Source/Charts/Mark/BalloonMarker.swift
* - Added optional in bundle to the viewFromXib() * Small styling fix * Keep only one method with Optional load from a bundle * Fixed new method signature in Obj-C Demo
* - Added optional in bundle to the viewFromXib() * Small styling fix * Keep only one method with Optional load from a bundle * Fixed new method signature in Obj-C Demo
* master: (55 commits) Refactors -[tableView:cellForRowAtIndexPath:] (ChartsOrg#3326) fix ChartsOrg#3311. Need one more key for iOS 11 camera roll saving revert a mistake, fix ChartsOrg#3299 add pie chart unit tests (ChartsOrg#3297) ChartsOrg#3287: align Objc and Swift demos balloon marker update changelog Min and Max reset when clearing ChartDataSet (Fixes ChartsOrg#3260) Restored old performance (ChartsOrg#3216) Support other bundle than main MarkerView.viewFromXib() (ChartsOrg#3215) For ChartsOrg#2840. add dataIndex parameter in `highlightValue()` calls (ChartsOrg#2852) Balloon Marker indicates position of data (ChartsOrg#3181) bump Info.plist version Fixed a duplicated assignment compared with obj-c code. (ChartsOrg#3179) Updated README for 3.0.5 (ChartsOrg#3183) BubbleChart uses correct colour for index now. Added custom text alignment for noData Fixes the distance issue between the legend and the horizontal bar chart (Fixes ChartsOrg#2138) (ChartsOrg#2214) call setNeedsDisplay() here to trigger render noDataText (ChartsOrg#3198) 添加定制TY的Mark 添加修改过的Mark到工程中 ... # Conflicts: # ICXCharts.podspec
Hi there;
why this pull request:
If you use the Charts Framework from another Bundle, this method will not find the ressources as this is looking in the main bundle only.
I added a method to be able to load the ressources from a given bundle.
Thanks,
Denis