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

Don't expand parent element if it's the root in single folder workspace when editing an element in explorer. #72627

Merged
merged 2 commits into from
May 24, 2019

Conversation

jeanp413
Copy link
Contributor

Fixes #72626

@octref octref requested a review from isidorn April 20, 2019 23:32
@isidorn
Copy link
Contributor

isidorn commented Apr 26, 2019

This trully fixes that issue.
However I have left some comments in your changes. Can you please address them.
Also why does this fix this issue? How did you find that this is the root cause?

@jeanp413
Copy link
Contributor Author

jeanp413 commented Apr 27, 2019

@isidorn Looking at this in more detail I found that the real problem is when disposing the input element because the renderElement method gets sometimes called more than once between this.tree.expand(e.parent!), this.refresh(false, e.parent); and this.tree.reveal(e);, and the ignoreDisposeAndBlur flag is still turned on because of the 100ms timeout. I'll update my PR with a better fix.

@jeanp413
Copy link
Contributor Author

This seems to be working except that it breaks when you are editing an element and click on the Refresh or Collapse All buttons, maybe they should be disabled like the New File and New Folder buttons.
Also I think ignoreDisposeAndBlur flag is no longer needed.

@isidorn isidorn added this to the May 2019 milestone May 7, 2019
@isidorn
Copy link
Contributor

isidorn commented May 24, 2019

This is good work. Let's merge it in. I will remove the ignoreDisposeAndBlur and check the refresh action. Thanks a lot!

@isidorn isidorn merged commit 89e5ea4 into microsoft:master May 24, 2019
@jeanp413
Copy link
Contributor Author

Great! Don't forget about the Collapse All action, it should also be disabled while editing.

@isidorn
Copy link
Contributor

isidorn commented May 27, 2019

Collapse All does not break anything for me. So I left it enabled.

@jeanp413
Copy link
Contributor Author

jeanp413 commented May 27, 2019

@isidorn I just build and tested it from source code, is this expected?
collapseall

@isidorn
Copy link
Contributor

isidorn commented May 28, 2019

You are correct. I have tackled that via 0991cb5
Thanks!

@orange4glace orange4glace mentioned this pull request Jun 18, 2019
@jeanp413 jeanp413 deleted the fix-72626 branch July 5, 2019 03:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dangling input elements after renaming top level file element in explorer view in single folder workspace
2 participants