Skip to content
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.

Sign up for GitHub

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

add legend format #2232

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add legend format #2232

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72a2047
add legend format
pkpatels Nov 27, 2017
2c4a21d
merge master
pkpatels Apr 15, 2018
47bdff9
make the changes asked
pkpatels Apr 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions c3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() :
typeof define === 'function' && define.amd ? define(factory) :
(global.c3 = factory());
typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You modified the distribution file!

You should modify the proper file from src directory and not commit the c3.js file which is overwritten during release

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just the indentation issue with my editor. I don't see any chages here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is that you modified the c3.js file which is the distribution file and that this file should not be part of the pull request.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@panthony i have make the requested changes. I have moved the logic to legend.js and config.js

You can review chnages.

typeof define === 'function' && define.amd ? define(factory) :
(global.c3 = factory());
}(this, (function () { 'use strict';

var CLASS = {
Expand Down Expand Up @@ -5097,6 +5097,7 @@ c3_chart_internal_fn.getDefaultConfig = function () {
legend_padding: 0,
legend_item_tile_width: 10,
legend_item_tile_height: 10,
legend_format: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, also add the documentation for this new option. The documentation site has been added to the repository.

// axis
axis_rotated: false,
axis_x_show: true,
Expand Down Expand Up @@ -7431,8 +7432,12 @@ c3_chart_internal_fn.updateLegend = function (targetIds, options, transitions) {
$$.api.revert();
}
});
l.append('text').text(function (id) {
return isDefined(config.data_names[id]) ? config.data_names[id] : id;
l.append('text').text(function (id, index) {
if(config.legend_format && isFunction(config.legend_format)) {
return config.legend_format(id, index);
} else {
return isDefined(config.data_names[id]) ? config.data_names[id] : id;
}
}).each(function (id, i) {
updatePositions(this, id, i);
}).style("pointer-events", "none").attr('x', $$.isLegendRight || $$.isLegendInset ? xForLegendText : -200).attr('y', $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendText);
Expand All @@ -7445,8 +7450,12 @@ c3_chart_internal_fn.updateLegend = function (targetIds, options, transitions) {
background = $$.legend.insert('g', '.' + CLASS.legendItem).attr("class", CLASS.legendBackground).append('rect');
}

texts = $$.legend.selectAll('text').data(targetIds).text(function (id) {
return isDefined(config.data_names[id]) ? config.data_names[id] : id;
texts = $$.legend.selectAll('text').data(targetIds).text(function (id, index) {
if(config.legend_format && isFunction(config.legend_format)) {
return config.legend_format(id, index);
} else {
return isDefined(config.data_names[id]) ? config.data_names[id] : id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, do not repeat the logic that resolve the legend's name.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get you here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkpatels I saw the same logic twice but it must have been between c3.js and legend.js files.

Your pull request still includes the c3.js file although it should not be committed.

}
} // MEMO: needed for update
).each(function (id, i) {
updatePositions(this, id, i);
Expand Down