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

7738-view-data-not-populated #9229

Merged

Conversation

danarad05
Copy link
Contributor

switching between view containers could cause view not to be populated with data

Signed-off-by: Dan Arad [email protected]

What it does

Fixes: #7738

How to test

  1. Load Theia when on Explorer tab and when "NPM scripts" section is expanded
    image
  2. before NPM scripts are populated with data, switch to "Search" tab and let it finish loading
  3. Return to "Explorer" tab - "NPM scripts" should populate shortly.

Review checklist

Reminder for reviewers

@danarad05
Copy link
Contributor Author

danarad05 commented Mar 22, 2021

FYI @amiramw @offer8

@vince-fugnitto vince-fugnitto added tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility labels Mar 22, 2021
@offer8
Copy link

offer8 commented Mar 25, 2021

hi @vince-fugnitto can we merge this one before the next release?
It solves us very high bug in multiple stakeholder scenarios.
We verified that the fix solved the issue.

@vince-fugnitto
Copy link
Member

hi @vince-fugnitto can we merge this one before the next release?

@offer8 I'll do my best to review but the pull-request is currently not approved. I confirmed it fixes the specific issue but I want to confirm that it is the proper solution. At the moment I see that the event is fired at all times, even if views are collapsed.

switching between view containers could cause view not to be populated with data

Signed-off-by: Dan Arad <[email protected]>
@danarad05 danarad05 force-pushed the 7738-view-data-not-populated branch from c1b8180 to 45630ec Compare March 25, 2021 16:53
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 confirmed that the pull-request fixes the issue, and that the original logic for expanded state looks correct 👍

@vince-fugnitto vince-fugnitto merged commit 5f57a3e into eclipse-theia:master Mar 25, 2021
@offer8
Copy link

offer8 commented Mar 25, 2021

thanks @vince-fugnitto can it be part of 1.12.0 https://github.com/eclipse-theia/theia/milestone/17?

@paul-marechal paul-marechal added this to the 1.12.0 milestone Mar 25, 2021
@offer8
Copy link

offer8 commented Mar 25, 2021

thanks @marechal-p !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm scripts tree is empty when activated while explorer view is not visible
4 participants