-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Support other bundle than main MarkerView.viewFromXib() #3215
Conversation
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.
Looks good. Just updated based on review and we'll merge for 3.1
@@ -72,10 +72,15 @@ open class MarkerView: NSUIView, IMarker | |||
} | |||
|
|||
@objc | |||
open class func viewFromXib() -> MarkerView? | |||
open class func viewFromXib() -> MarkerView? { |
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.
please put the {
on a new line.
} | ||
|
||
@objc | ||
open class func viewFromXib(in bundle: Bundle) -> MarkerView? |
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 can be one method: open class func viewFromXib(in bundle: Bundle = .main) -> MarkerView?
instead of two.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And keep the {
on a new line.
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.
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. |
@@ -72,10 +72,10 @@ open class MarkerView: NSUIView, IMarker | |||
} | |||
|
|||
@objc | |||
open class func viewFromXib() -> MarkerView? | |||
open class func viewFromXib(in bundle: Bundle = .main) -> MarkerView? |
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 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