Skip to content

Adding negative values on graphics bar#58

Open
bougetalife wants to merge 3 commits intotimqian:masterfrom
bougetalife:add_neg_part_to_bar
Open

Adding negative values on graphics bar#58
bougetalife wants to merge 3 commits intotimqian:masterfrom
bougetalife:add_neg_part_to_bar

Conversation

@bougetalife
Copy link
Copy Markdown

New Feature

We can use Bar chart with negative values
Option is added in order to obtain shorter labels on X axis. Label length remains the same on each bar when mouse is over.

Screenshot before and after this change

BEFORE
image

AFTER
image

@bougetalife bougetalife changed the title adding negative values to bar Adding negative values on graphics bar Dec 12, 2019
@timqian
Copy link
Copy Markdown
Owner

timqian commented Dec 13, 2019

Thanks, @bougetalife, the format seems not right, make sure you run npm run link and there is no error.

src/Bar.js Outdated
backgroundColor: 'white',
isShorterLabelActive: false,
charNum: 5,
shorterLabels: [],
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's focus on Adding negative values on bar chart for this PR first and not changing the options for bar chart.

Because:

  1. we need to make sure these options not only valid for bar but also other charts
  2. For shorter labels, adding 3 more options seems too overwhelming. We can discuss this in another issue or PR

src/Bar.js Outdated
.attr('height', (d) => this.height - yScale(d))
// .attr('y', (d) => yScale(d))
// .attr('height', (d) => this.height - yScale(d))
.attr('y', (d) => getRectY(this.height,d) ) ///////////////////////
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

unnecessary comments should be removed

@bougetalife
Copy link
Copy Markdown
Author

So for the first PR I add only negative values feature for Bar Chart : 8a93152

handling also x label position in case of negative values : ed8d2d8

Hope it answer your request !

@bougetalife bougetalife requested a review from timqian January 6, 2020 10:44
const yScale = getYScale(this.height, Math.min(...allData), Math.max(...allData))

function getYScale(height,min,max){
if (min < 0 && max > 0 )
Copy link
Copy Markdown
Owner

@timqian timqian Jan 7, 2020

Choose a reason for hiding this comment

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

@bikingbadger Sorry for the late reply

I see many formatting issues in this PR.

e.g. here is an unnecessary space

Can you lint the code via npm run lint, and fix the formatting issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants