| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 19
Added formatters #110
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
Added formatters #110
Conversation
Pull Request Test Coverage Report for Build 638
đź’› - Coveralls |
Prakriti-nith
force-pushed
the
formatters_google_datatables
branch
from
725fd80 to
5d6a9e8
Compare
|
In this PR, six formatters have been added: Arrow, Bar, Number, Date, Color, and Pattern. Out of these six formatters, Arrow and Bar are causing some issues and rest are working fine. |
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 write separate examples for all the formatters (and some combinations of formatters) in IRuby notebook and in web framework demo repo.
Attach the screenshot in PRs and clearly write the API in wiki page and PR to use this features.
| frmttr | ||
| end | ||
|
|
||
| def apply_remaining_formatter(formatter) |
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 be specific. Write down what are the formatters you are talking about in remaining. In future will it be able to handle other new formatters as well ?
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.
It works only for these six formatters as there are six separate classes for these formatters. We need to create a separate class for the new formatter.
| end | ||
| end | ||
|
|
||
| def apply_color_formatter(formatter) |
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 write one example, so that others can understand what are the options/parameter is required for this specific formatter and why there is separate method for this formatter.
Write down docs above all the methods.
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.
Done.
| end | ||
|
|
||
| def to_js | ||
| js = "\nvar formatter = new google.visualization.#{self.class.to_s.split('::').last}(" |
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.
Line length < 80
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.
Done
| context 'when formatters are applied in datatables' do | ||
| let(:data_tf) { | ||
| [ | ||
| ['Galaxy', 'Distance', 'Brightness', 'Galaxy-Distance', 'Date of discovery'], |
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.
line length < 80
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.
Done
| @@ -131,6 +131,110 @@ | |||
| expect(js).to match(/containerId: 'id'/) | |||
| expect(js).to match(/view: ''/) | |||
| end | |||
|
|
|||
| context 'when formatters are applied in datatables' do | |||
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 write separate testcases for each formatters, it will be easy to understand if one of the formatter is breaking .
I think formatters must contains all the information, so isn't we must check the values of it first?
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.
Done
If it is working in localhost then you must check what's the problem in online. For bar formatter attach screenshot of the examples. |
|
I will add rails examples in demo_daru-view and will update the wiki page. Rails example screenshot: |
Please response the comments during the last review, so that I can clear that it has been done. |
|
I have added the rails examples for this feature in this PR and IRuby notebook examples here. I will write the wiki page for formatters after this PR gets merged. |
| @@ -20,9 +21,13 @@ module Display | |||
| # @return [String] The ID of the DIV element that the Google Chart or | |||
| # Google DataTable should be rendered in | |||
| attr_accessor :html_id | |||
| attr_accessor :formatters | |||
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.
Why not in 23rd line itself ?
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.
html_id is documented on 21st and 22nd lines (above the attribute), so added attr_accessor :formatters in a separate line.
Added formatters for google datatables.