Skip to content

Conversation

@manuel-rubio
Copy link

Closes #91

@mindok
Copy link
Owner

mindok commented Jan 27, 2024

Thanks @manuel-rubio - I'll bundle this up with a fix to move the axis to the correct spot too.

@manuel-rubio
Copy link
Author

you're welcome, @mindok

I've just refactored the code to keep it simpler. I was unable (at the moment) to localise the part where the axis line is drawn. Could you shed light on where is the code in charge of that part?

@mindok
Copy link
Owner

mindok commented Feb 5, 2024

Hi @manuel-rubio - I was thinking about that over the weekend. I think the simplest approach is to just draw a zero line and leave the axes as they are. This would be done in Contex.BarChart.to_avg - there is currently a line to create the category axis (cat_axis_svg = if plot_options.show_cat_axis, do: Axis.to_svg(category_axis), else: "") - injecting something around there would work. e.g.

    zero_line = Axis.get_zero_line_svg(category_axis)

    [
      cat_axis_svg,
      val_axis_svg,
      zero_line,
      "<g>",
      get_svg_bars(plot),
      "</g>"
    ]

This will be pretty similar to Axisget_svg_gridlines for handling orientations etc, but with the line moved to the zero point.

So in axis.ex, we could have something like:

  def get_zero_line_svg(%Axis{scale: scale} = axis) do
    # Check that scale intersects zero by looking at domain
    # If so...
   domain_to_range_fn = Scale.domain_to_range_fn(scale)
   get_svg_gridline(axis, domain_to_range_fn.(0.0))
  end

@manuel-rubio
Copy link
Author

@mindok two things... I changed the code based on what you suggested and it's not working:

image

I was trying to run the tests and locally they're not working, but it's not broken because of my changes, are they working for you?

@mindok
Copy link
Owner

mindok commented Feb 7, 2024

Hi @manuel-rubio - the original tests rely on order of map keys (not a good idea). They broke with an upgrade of Elixir. It's on the to-do list...

I'll create a gallery item for the barchart that handles the zero case and see what's going on...

Could you maybe share the code that you used to generate the chart above.

@mindok
Copy link
Owner

mindok commented Feb 7, 2024

@manuel-rubio I made a basic bar chart with a negative value in the data based on the existing plain sample in the gallery. There are a couple of issues around labels etc that still need to be considered, and the value scale doesn't seem to reset.

Issues in the sample below:

  1. Data label doesn't render in the bar - the text location needs adjusting in a similar way to the bar. Your sample looks like it works ok.
  2. Value axis does not extend down far enough to show full negative value (the value should be -10.7). I'm not sure if you modified the value axis generation - your example above does it correctly.
  3. Bar renders over label on axis - I think this will be fixed when 2 is fixed as the category labels will be at the bottom of the plot.

Your sample looks a lot closer - are you happy to share the code used to generate it please. If you're happy for it to appear in the docs, I'll use it as a gallery example.

image

@manuel-rubio
Copy link
Author

@mindok my code is here: https://github.com/altenwald/conta/blob/main/apps/conta/lib/conta/stats.ex#L166-L196

It's in charge of giving you the data about Profits & Losses from your accounting via Telegram. It's still under development.

@manuel-rubio
Copy link
Author

Hi @manuel-rubio - the original tests rely on order of map keys (not a good idea). They broke with an upgrade of Elixir. It's on the to-do list...

If the order isn't important you can perform a Enum.sort previously to make the comparison, or create a assert_lists, something like:

defmacro assert_lists(a, b) do
  quote do
    assert Enum.sort(unquote(a)) == Enum.sort(unquote(b))
  end
end

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.

BarChart 0 is not 0

2 participants