WIP: Improvement: D3 Line Chart Styles and Formatting #652

2024-04-19T16:22:43+08:00

syedbarimanjan commented

2024-04-19 16:22:43 +08:00

(Migrated from github.com)

closes #643
/claim #643

Functional Requirements

Currently, we have a trendline_controller.js and a line_chart_controller.js. We should consolidate these into a single line_chart_controller that is configurable. Options will include:
- Hide or display x-axis labels ("trendlines" have no axis) - show by default
- Enable or disable tooltips ("trendlines" have no tooltip) - enable by default
When the chart is supplied with a single value or an "empty" Time Series (i.e. TimeSeries.new([])), it should display the "empty" state design.
The chart controller will always receive a TimeSeries.new([]).to_json value, which can be an instance of Money or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both Money values and regular integers (including percentages) in the tooltip.

Design Requirements

When the overall trend of the series that is passed to the line chart is "bad" (decreasing for an asset, increasing for a liability), the chart should go from a green color to a red color.
Ensure padding and spacing matches designs
When hovering over a data point in the chart, the line should highlight for dates prior to it, and the line after it should be grayed out
Implement gradient underneath the line (very subtle, see designs)
Refactor and improve overall styling

closes #643 /claim #643 ### Functional Requirements * [x] Currently, we have a `trendline_controller.js` and a `line_chart_controller.js`. We should consolidate these into a single `line_chart_controller` that is configurable. Options will include: * [x] Hide or display x-axis labels ("trendlines" have no axis) - show by default * [x] Enable or disable tooltips ("trendlines" have no tooltip) - enable by default * [x] When the chart is supplied with a **single value** or an "empty" Time Series (i.e. `TimeSeries.new([])`), it should display the "empty" state design. * [x] The chart controller will always receive a `TimeSeries.new([]).to_json` value, which can be an instance of `Money` or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both `Money` values and regular integers (including percentages) in the tooltip. ### Design Requirements * [x] When the _overall_ trend of the series that is passed to the line chart is "bad" (decreasing for an asset, increasing for a liability), the chart should go from a green color to a red color. * [x] Ensure padding and spacing matches designs * [x] When hovering over a data point in the chart, the line should highlight for dates prior to it, and the line after it should be grayed out * [x] Implement gradient underneath the line (very subtle, see designs) * [ ] Refactor and improve overall styling

syedbarimanjan commented

2024-04-19 16:26:22 +08:00

(Migrated from github.com)

The chart controller will always receive a TimeSeries.new([]).to_json value, which can be an instance of Money or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both Money values and regular integers (including percentages) in the tooltip.

@zachgoll can you further explain this.

* [ ] The chart controller will always receive a `TimeSeries.new([]).to_json` value, which can be an instance of `Money` or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both `Money` values and regular integers (including percentages) in the tooltip. @zachgoll can you further explain this.

zachgoll commented

2024-04-19 20:40:00 +08:00

(Migrated from github.com)

The chart controller will always receive a TimeSeries.new([]).to_json value, which can be an instance of Money or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both Money values and regular integers (including percentages) in the tooltip.

@zachgoll can you further explain this.

This basically just means that the D3 line chart should expect a TimeSeries data type as input, and since TimeSeries can hold multiple types of values (i.e. it can be a "money series" or just a "value series"), we need to make sure that our D3 chart can identify the value type and access the value properly.

The design file shows both a "Money Series" and "Percentage Series" (savings rate) as examples, so as long as the D3 chart appropriately handles both of these you should be all set.

> * [ ] The chart controller will always receive a `TimeSeries.new([]).to_json` value, which can be an instance of `Money` or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both `Money` values and regular integers (including percentages) in the tooltip. > > @zachgoll can you further explain this. This basically just means that the D3 line chart should expect a `TimeSeries` data type as input, and since `TimeSeries` can hold multiple types of values (i.e. it can be a "money series" or just a "value series"), we need to make sure that our D3 chart can identify the value type and access the value properly. The design file shows both a "Money Series" and "Percentage Series" (savings rate) as _examples_, so as long as the D3 chart appropriately handles both of these you should be all set.

syedbarimanjan commented

2024-04-19 21:42:05 +08:00

(Migrated from github.com)

The chart controller will always receive a TimeSeries.new([]).to_json value, which can be an instance of Money or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both Money values and regular integers (including percentages) in the tooltip.

@zachgoll can you further explain this.

This basically just means that the D3 line chart should expect a TimeSeries data type as input, and since TimeSeries can hold multiple types of values (i.e. it can be a "money series" or just a "value series"), we need to make sure that our D3 chart can identify the value type and access the value properly.

The design file shows both a "Money Series" and "Percentage Series" (savings rate) as examples, so as long as the D3 chart appropriately handles both of these you should be all set.

can you provide an example of percentage as where or how it will be used, there is a percent value in trend that will be the percent the chart will show and will it be like optional to make the chart pernctage one or if the percent value is not null it should make the chart parcentage?

> > * [ ] The chart controller will always receive a `TimeSeries.new([]).to_json` value, which can be an instance of `Money` or a regular integer (this is already implemented). As part of this issue, we need to make sure the chart can handle both `Money` values and regular integers (including percentages) in the tooltip. > > > > @zachgoll can you further explain this. > > This basically just means that the D3 line chart should expect a `TimeSeries` data type as input, and since `TimeSeries` can hold multiple types of values (i.e. it can be a "money series" or just a "value series"), we need to make sure that our D3 chart can identify the value type and access the value properly. > > The design file shows both a "Money Series" and "Percentage Series" (savings rate) as _examples_, so as long as the D3 chart appropriately handles both of these you should be all set. can you provide an example of percentage as where or how it will be used, there is a percent value in trend that will be the percent the chart will show and will it be like optional to make the chart pernctage one or if the percent value is not null it should make the chart parcentage?

zachgoll commented

2024-04-19 22:09:00 +08:00

(Migrated from github.com)

@syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it how to access and format the data points (this could also apply to Money values).

@syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it _how_ to access and format the data points (this could also apply to `Money` values).

syedbarimanjan commented

2024-04-19 23:16:25 +08:00

(Migrated from github.com)

@syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it how to access and format the data points (this could also apply to Money values).

I meant a real example of it being used somewhere in the software. If it isnt at the moment can you specify where it will be used so i can try to implement it or just use it with some mock data which will help me better understand it.

> @syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it _how_ to access and format the data points (this could also apply to `Money` values). I meant a real example of it being used somewhere in the software. If it isnt at the moment can you specify where it will be used so i can try to implement it or just use it with some mock data which will help me better understand it.

zachgoll commented

2024-04-19 23:46:47 +08:00

(Migrated from github.com)

@syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it how to access and format the data points (this could also apply to Money values).

I meant a real example of it being used somewhere in the software. If it isnt at the moment can you specify where it will be used so i can try to implement it or just use it with some mock data which will help me better understand it.

Got it, understood now. We do not have an implementation of this yet in the UI. I'd recommend creating a small placeholder series somewhere in the UI to test it. Something like:

<% series = TimeSeries.new([{ date: 2.days.ago.to_date, value: 88 }, { date: 1.day.ago.to_date, value: 89 }, { date: Date.today, value: 91 }]) %>
<div 
  data-controller="line-chart" 
  class="w-full h-full" 
  data-line-chart-series-value="<%= series.to_json %>"
>
</div>

> > @syedbarimanjan there is an example of a percent-based chart in the provided design file (last example) that shows how the tooltip and trend is displayed. To determine whether the chart displays a percentage value, I think we may want to add some sort of configuration option to the chart which tells it _how_ to access and format the data points (this could also apply to `Money` values). > > I meant a real example of it being used somewhere in the software. If it isnt at the moment can you specify where it will be used so i can try to implement it or just use it with some mock data which will help me better understand it. Got it, understood now. We do not have an implementation of this yet in the UI. I'd recommend creating a small placeholder series somewhere in the UI to test it. Something like: ```erb <% series = TimeSeries.new([{ date: 2.days.ago.to_date, value: 88 }, { date: 1.day.ago.to_date, value: 89 }, { date: Date.today, value: 91 }]) %> <div data-controller="line-chart" class="w-full h-full" data-line-chart-series-value="<%= series.to_json %>" > </div> ```

syedbarimanjan commented

2024-04-21 11:11:11 +08:00

(Migrated from github.com)

closing this in favour of #657

👍 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gavin/maybe#652