Skip to content

Tree Chart component - react-charting - Reflow #23895

Merged
AtishayMsft merged 70 commits intomicrosoft:masterfrom
Apurva-tech:usr/Apurva-tech/Reflow
Jul 14, 2022
Merged

Tree Chart component - react-charting - Reflow #23895
AtishayMsft merged 70 commits intomicrosoft:masterfrom
Apurva-tech:usr/Apurva-tech/Reflow

Conversation

@Apurva-tech
Copy link
Copy Markdown
Contributor

Verified that:

  • Code is up-to-date with the master branch
  • Your changes are covered by tests (if possible)
  • You've run yarn change locally

Current Behavior

No reflow and node adjustment

New Behavior

  • Add reflow and width slider
  • Adjust Node size and text value

Apurva-tech and others added 30 commits June 21, 2022 16:02
@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Jul 13, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4a00d7c:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam
Copy link
Copy Markdown
Collaborator

fabricteam commented Jul 13, 2022

📊 Bundle size report

🤖 This report was generated against 11a9b0c53578d9898926845678f13eeeb2441713

@size-auditor
Copy link
Copy Markdown

size-auditor bot commented Jul 13, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 11a9b0c53578d9898926845678f13eeeb2441713 (build)

this._linkElements = _linkElements;
}

public createEllipsisText(text: string, rectangleWidth: number, padding: number, styleClass: string | undefined) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename to truncateText

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

<text
textAnchor="middle"
className={metricName !== undefined ? this.styleClassNames.rectSubText : this.styleClassNames.rectText}
id={`${nodeId}`}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is the id value unique

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes it is unique for each node, but since we are not using selector anymore, this isn't needed

Fixed

* Node Width Size for the Tree Layout
* * @default 75
*/
layoutWidth?: NodesComposition.long | NodesComposition.compact | undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why using NodesComposition enum here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry, I was trying to take layoutWidth different for different composition earlier, then I changed the logic to make it common for both long/compact. Now we don't need it.

Fixed now

type="range"
value={this.state?._layoutWidth}
min={65}
max={90}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

have you defined the min value inside tree chart code as well. If less than 65 causes the tree to distort, then define this minimum value in the main code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

}
public createTree(givenLayoutWidth: number | undefined, screenWidth: number) {
if (givenLayoutWidth !== undefined && (givenLayoutWidth < 65 || givenLayoutWidth > 90)) {
givenLayoutWidth = 65;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if < 65 then set as 65. If more than 90 set as 90

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Apurva-tech Apurva-tech marked this pull request as ready for review July 14, 2022 13:57
@Apurva-tech Apurva-tech requested a review from a team as a code owner July 14, 2022 13:57
@AtishayMsft AtishayMsft merged commit 46367c1 into microsoft:master Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants