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

Fixing Tree chart issues #24752

Merged
merged 5 commits into from
Sep 27, 2022
Merged

Conversation

ankityadav4
Copy link
Contributor

@ankityadav4 ankityadav4 commented Sep 12, 2022

Current Behavior

  1. Parent node can only consist only of 2 lines of text.
  2. Chart appears asymmetric in layout i.e space from left and right side is unequal.

TreeBefore

New Behavior

  1. For adding the third optional text line in parent node. that is added as a optional body text. When the body text is given then the rectangle is removed.
    2.Secondly changes for alignment of the tree chart.
    a. So for this earlier the root node of the tree was made at screenwidth/3 which was creating the problem unequal padding from left and right.
    I have fixed it by making the root node to be made at screenwidth/2.
    b. Second thing was earlier the rectangles were not symmetric in nature, as the rectangles were made using the x y coordinate, instead the x and y coordinates must be taken as centre coordinates, so that when layout width (rectangle width) changes the chart always remain symmetric.

TreeAfter

Related Issue(s)

Fixes #

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 12, 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 3c0d44a:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

📊 Bundle size report

🤖 This report was generated against af37ad65b2943761d035c10d14c87912b71f73ce

@size-auditor
Copy link

size-auditor bot commented Sep 12, 2022

Asset size changes

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

Baseline commit: af37ad65b2943761d035c10d14c87912b71f73ce (build)

</>
) : (
<tspan className={this.styleClassNames.rectSubText} dy="1.4em" x={xCoordinate + rectangleWidth / 2}>
{subname}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is subname twice here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we are checking all composition , subname is twice because firstly we are checking for if optional bodyname is there then we are placing it below subname and for last case when it is not there so there will only be subname.

@@ -172,12 +182,11 @@ class StandardTree {
): string {
// gap adds ratio for parent.y to child.y

const path = `M${childX},${childY - gap} H${childXMax + rectWidth} M${parentX + rectWidth / 2},${childY - gap}
const path = `M${childX - rectWidth / 2},${childY - gap} H${childXMax + rectWidth / 2} M${parentX},${childY - gap}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add small comments with examples of the computations here. This is will help someone not familiar with the logic to understand it better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the comments for this.

@ankityadav4 ankityadav4 marked this pull request as ready for review September 27, 2022 15:29
@ankityadav4 ankityadav4 requested a review from a team as a code owner September 27, 2022 15:29
@ankityadav4 ankityadav4 merged commit d4e6367 into microsoft:master Sep 27, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 28, 2022
* master: (21 commits)
  chore: Migrate react-avatar to use new build (microsoft#24969)
  applying package updates
  chore(react-input, react-textarea): Deprecating filled with shadow appearance variants (microsoft#24900)
  fix: v8 Dropdown no longer sets incorrect and unnecessary aria-activedescendant (microsoft#24593)
  feat: v0 Tooltip migration from v9 (microsoft#24908)
  chore: bump devDeps to fix critical security vulnerability (microsoft#24891)
  Fixing Tree chart issues (microsoft#24752)
  init: new package react-avatar-context (microsoft#24968)
  ci(.github): add issues write permisions to triage-bot worflow (microsoft#24963)
  applying package updates
  fix(Toolbar): close previous submenu when opening another submenu (microsoft#24836)
  fix: update non-focus-trap Popover role to be group (microsoft#24897)
  feat: Avatar's aria label includes 'active' or 'inactive' when using the active prop (microsoft#24901)
  feat(scripts): implement triage-bot module (microsoft#24911)
  chore: bump @octokit/rest to v18 (microsoft#24919)
  stress test: add "build-fixture" command (microsoft#24928)
  BREAKING-CHANGE: new ChatMessageContent for style caching (microsoft#24691)
  bugfix: fix changefile to properly update version of react-components with a patch (microsoft#24949)
  feat(scripts): enable strict checking for additional sub-folders(packages) (microsoft#24526)
  chore: exports DialogContent as unstable (microsoft#24943)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
* Fixing Tree chart issues for adding the optional bodyText for parent node and adjusting the allignment of the tree in the screen

* Adding change file

* Refactoring code

* Refactoring code

* Resolving review comments
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