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

fix(declarative-chart): Handle 10^N yMax case for rounded ticks #33870

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Anush2303
Copy link
Contributor

New Behavior

image

Related Issue(s)

  • Fixes #

@Anush2303 Anush2303 marked this pull request as ready for review February 19, 2025 12:27
@Anush2303 Anush2303 requested a review from a team as a code owner February 19, 2025 12:27
Copy link

📊 Bundle size report

✅ No changes found

Copy link

Pull request demo site: URL

@@ -0,0 +1,7 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual regressions to review in the fluentuiv8 Visual Regression Report

react-charting-LineChart 1 screenshots
Image Name Diff(in Pixels) Image Type
react-charting-LineChart.Gaps.default.chromium.png 1 Changed

return Math.log10(n) % 1 === 0;
}

function roundIfAlmostInteger(num: number): number {
Copy link
Contributor

Choose a reason for hiding this comment

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

roundIfAlmostInteger

Call this handleFloatingPointPrecisionError and add link to this article as comment.
https://docs.python.org/release/2.5.1/tut/node16.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay.

@@ -256,6 +256,15 @@ function multiFormat(date: Date, locale?: d3TimeLocaleObject, useUTC?: boolean)
)(date);
}

function isPowerOf10(n: number): boolean {
return Math.log10(n) % 1 === 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

=== 0

instead of 0 it should be within floating point precision error

Copy link
Contributor Author

@Anush2303 Anush2303 Feb 19, 2025

Choose a reason for hiding this comment

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

We are already checking that in 'handleFloatingPointPrecisionError' function. Then is it needed here also? Rounded value will only come here.

@@ -391,6 +391,11 @@ describe('prepareDatapoints for rounded tick value cases', () => {
const result = utils.prepareDatapoints(589030.78, -234.45, 4, false, true);
matchResult(result);
});

it('should return an array with rounded data points when yMax is almost power of 10', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

almost

instead call this power of 10 with floating point precision error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay.

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.

3 participants