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.

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

Feature: Force morris to use only integer values on Y axis #344

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

t3chn0r
Copy link

@t3chn0r t3chn0r commented Dec 24, 2013

New option gridIntegers was added to force morris to use only integer numbers on the grid (Y axis). gridIntegers accepts a boolean value (false=default). When set to true, morris will use only integer values on the Y axis.

New option gridIntegers was added to force morris to use only integer numbers on the grid (Y axis). gridIntegers accepts a boolean value (false=default). When set to true, morris will use only integer values on the Y axis.
@sudodoki
Copy link
Contributor

To be more sure that everything works fine, try creating few testcases in
[https://github.com/oesmith/morris.js/tree/master/spec/lib/grid](grid test folder) and maybe adding an example page for clearer visual result. I'm not an authority here, but these steps will make it more probable and easier to integrate into library.
No need to check x is true for boolean variable, but that really depends on your code style.

@chilipepper987
Copy link

This worked out great for me! With one caveat... it only seems to to take affect if you specify custom y boundaries. I would like to still get integer gridlines using auto y boundaries. To achieve that, just after the if block on line 313 that begins with

if (((_ref = this.options.axes) === true || _ref === 'both' || _ref === 'y') || this.options.grid === true) {

I added this to mine to ensure that the grid lines are still integers on auto y boundaries

     //we still need to round gridlines
      if (this.options.gridIntegers)
      {
          for (var i=0;len=this.grid.length,i<len;i++)
          {
              this.grid[i]=Math.round(this.grid[i]);
          }
      }

this might be more appropriate to be put in the autoGridLines method, but putting it there gets the job done for me.

PS looking through the code made me aware of the options.numLines parameter, which is very useful! But I didn't see it mentioned on the main API doc page
http://www.oesmith.co.uk/morris.js/lines.html

@edumucelli
Copy link

That works for me. As @chilipepper987 said, it works when I've set ymin boundary. In my case it is ok because I know I can set ymin as 0. Great!

@MichaelKubovic
Copy link

Unfortunately, I must say that it doesn't work as expected for some datasets. For example following data results in step being 0 which further results in errors in rendering...
Error: Invalid value for attribute width="Infinity"

1692: [
    {time: '0 - 5 sec', views: 1},
    {time: '6 - 10 sec', views: 1},
    {time: '11 - 15 sec', views: 0},
    {time: '16 - 20 sec', views: 0},
    {time: '21+ sec', views: 0},
],

My config:

Morris.Bar({
    element: 'popChart' + slide_id,
    data: popChartsData[ slide_id ],
    xkey: 'time',
    ykeys: ['views'],
    labels: ['Leads viewed'],
    xLabelMargin: 10,
    hideHover: 'auto',
    barColors: ["#3d88ba"],
    ymin: 0,
    gridIntegers: true
});

I've managed to fix it by simply enforcing the step to be at least 1.

if(this.options.gridIntegers === true) {
      step = Math.min(1, Math.round(step));
      step = step === 0 ? 1 : step;
}

@rgravina rgravina mentioned this pull request Jul 22, 2014
@rzds
Copy link

rzds commented Aug 8, 2014

this does not work if you don't set at least ymin.
it shouldn't do that.

@marcomorain
Copy link

👍

pierresh pushed a commit to pierresh/morris.js that referenced this pull request Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants