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

barista: Modifying isChild condition for avoiding opening wrong nodes #1351

Conversation

PedroMosquera
Copy link
Contributor

@PedroMosquera PedroMosquera commented Jul 22, 2020

Pull Request

Fixes #1245

The current sunburst uses IDs as a representation for each node. You can easy see the parent of every child by the base of their ID.

e.g.: node with ID 1.3.1 is the child of the node with 1.3.

The current issue is that the application returns true when you ask if 1.12.1 is the child of 1.1

Type of PR

Bugfix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING doc and I follow the PR guidelines
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@tomheller
Copy link
Collaborator

Hi @PedroMosquera! Thank you for this contribution the code looks good. Would it make sense to add a test case for this, so we can make sure that this behaviour will not break in the future?

@PedroMosquera
Copy link
Contributor Author

Sure thing @tomheller, it totally makes sense. I'm working on those now 😄

lukasholzer
lukasholzer previously approved these changes Jul 23, 2020
@lukasholzer lukasholzer self-requested a review July 23, 2020 08:02
@lukasholzer lukasholzer marked this pull request as draft July 23, 2020 11:59
@PedroMosquera PedroMosquera marked this pull request as ready for review July 23, 2020 14:07
@lukasholzer
Copy link
Contributor

@PedroMosquera please rebase with the latest master as there is a fix why build, stylelint and unit_test is failing

@PedroMosquera PedroMosquera force-pushed the barista/sunburst-chart-opening-additional-children branch 2 times, most recently from 60973f1 to f7341fc Compare July 24, 2020 08:31
@PedroMosquera
Copy link
Contributor Author

@lukasholzer I've just rebased master and those tasks are still failing. Not sure if I have to do something more

@tomheller tomheller force-pushed the barista/sunburst-chart-opening-additional-children branch from f7341fc to 3308198 Compare July 27, 2020 12:16
@tomheller
Copy link
Collaborator

Hi @PedroMosquera!
I have updated your PR from master (included an additional fix for the bazel checks) and have squashed the commits as this now look rather good.
Thank you for this contribution 🎉

@tomheller tomheller added pr: needs-cherry-pick When a pull request needs manual cherry picking target: minor This PR is targeted for the next minor release target: patch This PR is targeted for the next patch release labels Jul 27, 2020
@tomheller tomheller merged commit 6da1754 into dynatrace-oss:master Jul 27, 2020
@tomheller
Copy link
Collaborator

Cherry pick done.

@PedroMosquera PedroMosquera deleted the barista/sunburst-chart-opening-additional-children branch March 22, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: needs-cherry-pick When a pull request needs manual cherry picking target: minor This PR is targeted for the next minor release target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sunburst chart opening additional children
4 participants