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.

Sign up for GitHub

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

Jump to bottom

ensure that on expansion nodes are refreshed #7400

Merged
merged 1 commit into from
Mar 23, 2020
Merged

ensure that on expansion nodes are refreshed #7400

merged 1 commit into from
Mar 23, 2020

Conversation

akosyakov
Copy link
Member

What it does

How to test

  • there is a unit test which fails on master
  • test reveal in the explorer command
  • test expansion in different trees

Review checklist

Reminder for reviewers

@akosyakov akosyakov added the tree issues related to the tree (ex: tree widget) label Mar 23, 2020
jankeromnes
jankeromnes approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Setting node.expanded = true after having actually done the job makes a lot of sense to me.

Also, I've confirmed that the test actually fails on master, but now passes with your change.

vince-fugnitto
vince-fugnitto approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I verified that the test fails on master but passes with your changes.
I also verified that 'reveal in explorer', expanding/collapsing nodes, and collapse all still work as intended.

@akosyakov akosyakov merged commit cc9453d into master Mar 23, 2020
@akosyakov akosyakov deleted the akosyakov/expanded-node-does-not-7399 branch March 23, 2020 14:57
@marcdumais-work marcdumais-work restored the akosyakov/expanded-node-does-not-7399 branch March 23, 2020 19:11
@marcdumais-work marcdumais-work deleted the akosyakov/expanded-node-does-not-7399 branch March 23, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vince-fugnitto vince-fugnitto vince-fugnitto approved these changes

@jankeromnes jankeromnes jankeromnes approved these changes

@elaihau elaihau Awaiting requested review from elaihau

@svenefftinge svenefftinge Awaiting requested review from svenefftinge

Assignees
No one assigned
Labels
tree issues related to the tree (ex: tree widget)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

expanded node does not ensure that node is refreshed
3 participants
@akosyakov @jankeromnes @vince-fugnitto