Skip to content

Refactor/improve standards#13

Open
Kuncheria-KV wants to merge 12 commits intomasterfrom
refactor/improve-standards
Open

Refactor/improve standards#13
Kuncheria-KV wants to merge 12 commits intomasterfrom
refactor/improve-standards

Conversation

@Kuncheria-KV
Copy link
Copy Markdown

  • Change a prop name to make the component more generic
  • Fix type definitions, that were not exported in the final build

Comment thread PULL_REQUEST_TEMPLATE.md Outdated
Comment thread STYLE_GUIDELINES.md Outdated
…er states, refactored waterfallchart component, util and hook files, fixed an issue where component was getting stuck when value 0 is passed for yAxisPixelsPerUnit
@arjunrtk-kv arjunrtk-kv force-pushed the refactor/improve-standards branch from 1b3dc4e to 4587bd0 Compare November 30, 2023 08:56
Comment thread package.json Outdated
{
"name": "@keyvaluesystems/react-waterfall-chart",
"version": "0.1.5",
"version": "1.0.0",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No need to change the version manually. There is an option to do it during deployment.Please revert these changes

Comment thread README.md


The transactions prop is an array of transactions with the following keys:
The dataPoints prop is an array of dataPoint with the following keys:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since this prop renaming is a breaking change, it would be better to specify it in the readme along with the migration steps. For more info refer other repos that has similar changes

Comment thread README.md
Comment thread README.md Outdated
Comment thread src/tests/waterfallChart.test.tsx
Comment thread src/waterfall-chart/WaterFallChart.tsx
}

export function checkIfScaleSufficient(scale: number, maxLabelsCount: number, valueRange: number): boolean {
if (maxLabelsCount === 0) return true; // to stop the while loop from checking for sufficient scale with zero maxLabelsCount
Copy link
Copy Markdown

@ReshmaJoshy ReshmaJoshy Jan 17, 2024

Choose a reason for hiding this comment

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

Removed the line since it is unreachable code; the case where maxLabelsCount = 0 is already addressed earlier in the code.

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.

4 participants