Html legend dc v3 #1392

kum-deepak · 2018-03-30T17:40:33Z

Based on #1329. Updated for D3v4.

kum-deepak · 2018-03-30T17:54:50Z

@rrameshkumar76 Please review changes.

rrameshkumar76

Thanks for considering this PR for merge. Looks good to me. I don't have any experience using D3v4, Looks like these are just method changes for v4 in specs.
The only one I am not sure about is the usage of .nodes() for [0].

kum-deepak · 2018-03-31T02:40:48Z

Actually quite a lot has changed between D3v3 to D3v4. Thankfully in your code there was nothing that needed change :) The [0] needs to be changed to .nodes() in D3v4.

I have not gone very deeply into the functionality of the widget, I do understand that it offers features that are different from the legend already included in the library. Please add one/two more example that demonstrates those.

I have added you as a contributor to my fork of dc.js. Please checkout branch html-legend-dc-v3 and push your changes there.

Documentation will also need to be updated.

rrameshkumar76 · 2018-03-31T12:38:42Z

Some of the reasons to add the html legend are discussed in issue #1325
The main reason I use is for scroll bars(#1300) and for other users to override the legend styles using css. I did get a scroll implemented in my project for the svg legend but find this much cleaner.

kum-deepak · 2018-03-31T12:45:19Z

Thanks @rrameshkumar76, can you please add an example showing scrollbar and css styling. I have given you commit access to the repository, you can directly commit to the branch.

rrameshkumar76 · 2018-03-31T13:56:51Z

I have updated the example for html legends to show the scrollbar.Please take a look and let me know if it needs more updates.

Another feature I can add is to highlight the selected legend. I have that working in my project without specs which i would need to add. I can commit that to the same branch.

kum-deepak · 2018-03-31T15:26:37Z

Please add the highlight feature and commit to the same branch.

rrameshkumar76 · 2018-04-01T00:11:17Z

I have added the highlight feature. Please take a look in the example. specs are pending. If It looks good then will go ahead and add the specs.

The build fails in "saucelabs-jasmine:all" . I hope this has nothing to do with my commit?

kum-deepak · 2018-04-01T07:19:58Z

Thanks @rrameshkumar76, the example looks much more explanatory now. The feature to highlight selected items is definitely useful. I have made minor changes in CSS (dc.scss and the example html) - just to make example look better. Please pull before making further changes.

Please go ahead with the specs. Also check the documentation at http://localhost:8888/web/examples/html-legend.html, some methods have no documentation as of now.

Ignore the errors on "saucelabs-jasmine:all", it is not related to your commits. Sauce labs is a 3rd party service to run tests on multiple browsers. In this repository credentials for that service are not there so it will fail.

kum-deepak · 2018-04-01T07:21:58Z

src/html-legend.js

@@ -16,7 +14,8 @@ dc.htmlLegend = function () {
        _container,
        _legendText = dc.pluck('name'),
        _maxItems,
-        _horizontal = false;
+        _horizontal = false,
+        _legendItemClass;

    var _l;


Can this be made a local variable in _legend.render?

Do you mean the _legendItemClass variable? This can be overridden using the legendItemClass method in below file.
The legend highlighting is probably only useful/works for pie chart legends and not for multi line charts/stacked bar charts as the behaviour of legend usage is different in those charts.
One thing good may be to add the legend highlight as optional parameter(highlightSelected) allowing user to choose.

Sorry I meant the var _l

Yes, I also agree that making the highlight selection as optional setting is a good idea. Thanks!

will move _l to be a a local variable and add highlightSelected as optional setting.

kum-deepak · 2018-04-01T15:56:53Z

Thanks @rrameshkumar76, I will check and get back. 👍

rrameshkumar76 · 2018-04-01T16:18:43Z

documentation update and spec for highlight is still pending. I think i will need to add a piechart and attach htmlLegend in specs for the unit testing the highlightSelected option.

kum-deepak · 2018-04-02T13:55:50Z

@rrameshkumar76 I have made some code changes which should not have changed any functionality. dc.js charts generally keep CSS class names as variables. Also simplified the code using using d3's classed. See if you are happy with the changes.

Thanks for your great work!

rrameshkumar76 · 2018-04-03T01:24:03Z

The changes look good. I fixed the jsdoc , spec is still pending.

gordonwoodhull · 2018-04-03T22:03:30Z

Thanks @rrameshkumar76! I look forward to reviewing this week. Going forward, I want to figure out a plan for keeping the HTML legend in sync with the SVG legend. I don't think code or tests can be shared, so maybe it is just something to keep in mind for reviews.

@kum-deepak, learning how to write (and fix) specs is often a blocker for contributors. I don't think the difficulty is Jasmine so much as understanding what it means to test the DOM. Since you've already picked this up, maybe you have suggestions for how to write the specs for this PR?

(There are also quite a few PRs which look good except for missing/broken specs. If you're interested in helping in this area, we could discuss it in the #1391 discussion thread.)

kum-deepak · 2018-04-04T16:56:20Z

Thanks @rrameshkumar76! Next 2-3 days I will not be responding frequently. In the meanwhile please check the API of SVG Legend to ensure that this module implements a super-set of the functionality. Same goes for specs as well. Again many thanks for you diligent efforts.

rrameshkumar76 · 2018-04-04T22:24:03Z

The html legend should already have all the features of the svg legend with some extra functionality added to it. I just have one spec left for one of the methods (legendItemClass). Other specs have been completed.

…dded example for horizontal menu.

kum-deepak · 2018-04-22T08:06:29Z

Changed the base to dc-js:develop

gordonwoodhull · 2018-04-28T16:55:29Z

Thanks @davejlong, @rrameshkumar76, @kum-deepak!

Merged for 3.0.0

kum-deepak mentioned this pull request Mar 30, 2018

Add Html legend support #1329

Closed

rrameshkumar76 reviewed Mar 30, 2018

View reviewed changes

kum-deepak commented Apr 1, 2018

View reviewed changes

gordonwoodhull closed this Apr 20, 2018

gordonwoodhull reopened this Apr 20, 2018

Ramesh Rajagopalan and others added 10 commits April 22, 2018 09:00

Add HTML legend support

35e0deb

Add maxItems and legendText for HTML legends

c3580be

fix comment

ab86095

Updated for D3v4

08a969a

updated html legend example to show scrollbar for vertical menu and a…

57e389c

…dded example for horizontal menu.

updated html legend to highlight the selected legend items.

7e3508a

Updated html legend example page.

4f23116

CSS changes to make example look better

d57ab42

added highlightSelected option to html legend.

d752ab6

Minor code changes

08a9979

kum-deepak changed the base branch from 3.0 to develop April 22, 2018 08:05

gordonwoodhull mentioned this pull request Apr 28, 2018

Html legend #577

Closed

gordonwoodhull closed this Apr 28, 2018

kum-deepak deleted the html-legend-dc-v3 branch April 28, 2018 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Html legend dc v3 #1392

Html legend dc v3 #1392

kum-deepak commented Mar 30, 2018

kum-deepak commented Mar 30, 2018

rrameshkumar76 left a comment

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Mar 31, 2018 •

edited

Loading

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Mar 31, 2018

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Apr 1, 2018 •

edited

Loading

kum-deepak commented Apr 1, 2018

kum-deepak Apr 1, 2018

rrameshkumar76 Apr 1, 2018 •

edited

Loading

kum-deepak Apr 1, 2018

rrameshkumar76 Apr 1, 2018 •

edited

Loading

kum-deepak commented Apr 1, 2018

rrameshkumar76 commented Apr 1, 2018

kum-deepak commented Apr 2, 2018

rrameshkumar76 commented Apr 3, 2018

gordonwoodhull commented Apr 3, 2018

kum-deepak commented Apr 4, 2018

rrameshkumar76 commented Apr 4, 2018 •

edited

Loading

kum-deepak commented Apr 22, 2018

gordonwoodhull commented Apr 28, 2018

Html legend dc v3 #1392

Html legend dc v3 #1392

Conversation

kum-deepak commented Mar 30, 2018

kum-deepak commented Mar 30, 2018

rrameshkumar76 left a comment

Choose a reason for hiding this comment

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Mar 31, 2018 • edited Loading

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Mar 31, 2018

kum-deepak commented Mar 31, 2018

rrameshkumar76 commented Apr 1, 2018 • edited Loading

kum-deepak commented Apr 1, 2018

kum-deepak Apr 1, 2018

Choose a reason for hiding this comment

rrameshkumar76 Apr 1, 2018 • edited Loading

Choose a reason for hiding this comment

kum-deepak Apr 1, 2018

Choose a reason for hiding this comment

rrameshkumar76 Apr 1, 2018 • edited Loading

Choose a reason for hiding this comment

kum-deepak commented Apr 1, 2018

rrameshkumar76 commented Apr 1, 2018

kum-deepak commented Apr 2, 2018

rrameshkumar76 commented Apr 3, 2018

gordonwoodhull commented Apr 3, 2018

kum-deepak commented Apr 4, 2018

rrameshkumar76 commented Apr 4, 2018 • edited Loading

kum-deepak commented Apr 22, 2018

gordonwoodhull commented Apr 28, 2018

rrameshkumar76 commented Mar 31, 2018 •

edited

Loading

rrameshkumar76 commented Apr 1, 2018 •

edited

Loading

rrameshkumar76 Apr 1, 2018 •

edited

Loading

rrameshkumar76 Apr 1, 2018 •

edited

Loading

rrameshkumar76 commented Apr 4, 2018 •

edited

Loading