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

Area with baseline chart #813

Merged
merged 11 commits into from
Nov 2, 2021

Conversation

thanhlmm
Copy link

@thanhlmm thanhlmm commented Aug 5, 2021

Type of PR: enhancement

PR checklist:

Overview of change:
Add new api to add Area with baseline

	const mainSeries = chart.addAreaBaselineSeries({
		lineWidth: 1,
		baseValue: {
			type: 'price',
			price: 0,
		},
	});

Is there anything you'd like reviewers to focus on?

@thanhlmm
Copy link
Author

thanhlmm commented Aug 5, 2021

It still missing custom style for line top and bottom now. I think it can be postponed

@thanhlmm
Copy link
Author

thanhlmm commented Sep 6, 2021

@timocov Any thought on this PR?

@timocov
Copy link
Contributor

timocov commented Sep 8, 2021

Note for others who might be concerned: we discussed with @thanhlmm in PM this PR and agreed that I'll push some changes to this PR and provide an update what is changed. The main reason of this is that we already have a code for baseline chart on tradingview.com so it would be easy for us to support similar code rather than have several implementation of the same things. We scheduled this feature for 3.7 version.

- Fixed docs
- Updated default colors
- Added lastPriceAnimation to baseline series
- Refactored pane renderers and pane views (since baseline is almost area we can re-use almost all code from area)
@timocov
Copy link
Contributor

timocov commented Nov 2, 2021

Hi there @thanhlmm,

I just pushed changes to your branch as we discussed previously and here is a list of main changes against yours (luckily no breaking changes in API):

  • resolved conflicts with master branch
  • updated default values of colors for baseline series
  • updated docs
  • refactored pane renderers and pane views since baseline looks pretty similar to area with the only change of fill/stroke style, so I extracted that to the base class
  • corrected test cases
  • removed adding 0.01 to the gradient step for the bottom part (it seems it works fine now, did you have any issue with that?)

Let me know what do you think.

Also, I should say that I didn't change a lot actually so you did really good job. Thanks for your contribution!

@timocov timocov assigned timocov and thanhlmm and unassigned timocov and thanhlmm Nov 2, 2021
@timocov timocov enabled auto-merge November 2, 2021 12:07
@timocov timocov mentioned this pull request Nov 2, 2021
@timocov timocov disabled auto-merge November 2, 2021 12:08
@timocov timocov merged commit 5276863 into tradingview:master Nov 2, 2021
@thanhlmm
Copy link
Author

thanhlmm commented Nov 3, 2021

Awesome

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.

Baseline series type
2 participants