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

Merged
merged 8 commits into from Jul 29, 2018

Conversation

Projects
None yet
3 participants
@Prakriti-nith
Contributor

Prakriti-nith commented Jul 19, 2018

Added formatters for google datatables.

@coveralls

This comment has been minimized.

coveralls commented Jul 19, 2018

Pull Request Test Coverage Report for Build 638

  • 117 of 118 (99.15%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.0%) to 98.102%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/daru/view/adapters/googlecharts/display.rb 39 40 97.5%
Totals Coverage Status
Change from base Build 637: 1.0%
Covered Lines: 1499
Relevant Lines: 1528

💛 - Coveralls

Prakriti-nith added some commits Jul 19, 2018

@Prakriti-nith Prakriti-nith force-pushed the Prakriti-nith:formatters_google_datatables branch from 725fd80 to 5d6a9e8 Jul 20, 2018

@Prakriti-nith Prakriti-nith changed the title from [WIP] Added formatters to Added formatters Jul 20, 2018

@Prakriti-nith

This comment has been minimized.

Contributor

Prakriti-nith commented Jul 21, 2018

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.
Bar formatter is working correctly in rails but in IRuby notebook it generates weird bars. Arrow formatter is neither generating arrows in nbviewer (working fine in localhost) nor in rails. Examples.

@Shekharrajak

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)

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 22, 2018

Collaborator

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 ?

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 22, 2018

Contributor

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)

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 22, 2018

Collaborator

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.

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 25, 2018

Contributor

Done.

end

def to_js
js = "\nvar formatter = new google.visualization.#{self.class.to_s.split('::').last}("

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 22, 2018

Collaborator

Line length < 80

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 25, 2018

Contributor

Done

context 'when formatters are applied in datatables' do
let(:data_tf) {
[
['Galaxy', 'Distance', 'Brightness', 'Galaxy-Distance', 'Date of discovery'],

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 22, 2018

Collaborator

line length < 80

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 25, 2018

Contributor

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

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 22, 2018

Collaborator

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?

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 25, 2018

Contributor

Done

@Shekharrajak

This comment has been minimized.

Collaborator

Shekharrajak commented Jul 22, 2018

Bar formatter is working correctly in rails but in IRuby notebook it generates weird bars. Arrow formatter is neither generating arrows in nbviewer (working fine in localhost) nor in rails

If it is working in localhost then you must check what's the problem in online. For bar formatter attach screenshot of the examples.

@Prakriti-nith

This comment has been minimized.

Contributor

Prakriti-nith commented Jul 22, 2018

I will add rails examples in demo_daru-view and will update the wiki page.
Updated examples.
PR of rails examples in demo_daru-view.
@Shekharrajak can you please review this PR?

Rails example screenshot:
bars1
bars2

@Shekharrajak

This comment has been minimized.

Collaborator

Shekharrajak commented Jul 25, 2018

@Shekharrajak can you please review this PR?

Please response the comments during the last review, so that I can clear that it has been done.

@Prakriti-nith

This comment has been minimized.

Contributor

Prakriti-nith commented Jul 25, 2018

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.
I have attached the screenshot for bar formatter. Regarding Arrow formatter, it works in rails app when we use loader.js to draw the charts rather than jsapi but again that causes problems in IRuby notebook. I think the additional required code is already loaded in jupyter notebook that is why it is working on localhost. I am still searching why it is not working online.

@@ -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

This comment has been minimized.

@Shekharrajak

Shekharrajak Jul 28, 2018

Collaborator

Why not in 23rd line itself ?

This comment has been minimized.

@Prakriti-nith

Prakriti-nith Jul 28, 2018

Contributor

html_id is documented on 21st and 22nd lines (above the attribute), so added attr_accessor :formatters in a separate line.

@Shekharrajak Shekharrajak merged commit 11cb020 into SciRuby:master Jul 29, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.0%) to 98.102%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment