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

Fix missing commit and amend widgets on opening new workspace #7769

Merged
merged 1 commit into from
May 12, 2020

Conversation

westbury
Copy link
Contributor

@westbury westbury commented May 7, 2020

Fixes #7734

The issue was caused because the commit and amend components were being rendered before the repository was set. Adding the repository listener in itself did not fix the problem. The reason is that the widget may be flagged as hidden when the update occurs.

One might think that updating the widgets by inserting this.commitWidget.update() after this.commitWidget.show() in scm-widget would work. However the 'show' is done asynchronously and the widget is still marked as hidden when the onUpdateRequest is called. Thus the most reliable solution is to simply remove the test for isVisible (and isAttached).

There is a further change to also remove this test in ScmWidget. If that is not done then the ScmNoRepositoryWidget can fail to appear, resulting in a blank view instead of the Alert.

Unrelated to the issue, I have also removed a couple of unused injections.

How to test

Test the condition detailed in the issue. Test around the whole process of switching repositories and workspaces. Check the Alert shows when no repository.

Review checklist

Reminder for reviewers

@westbury westbury added the scm issues related to the source control manager label May 7, 2020
@westbury westbury force-pushed the fix-missing-widgets branch from 3807c58 to 1f27efa Compare May 7, 2020 14:15
@akosyakov
Copy link
Member

I am off tomorrow. It would be good if someone review it.

@lmcbout
Copy link
Contributor

lmcbout commented May 7, 2020

Testing it right now on UBUNTU. If I put the focus on the SCM GIT view , opening a new view, closing + re-opening a view with / without GIT, after a few switch , the focus does not remain on the SCM GIT view. I need to re-select it to see the data.
The ALERT shows and the AMEND button also. Seems to work fine

@westbury
Copy link
Contributor Author

westbury commented May 8, 2020

If I put the focus on the SCM GIT view , opening a new view, closing + re-opening a view with / without GIT, after a few switch , the focus does not remain on the SCM GIT view. I need to re-select it to see the data.

If I understand correctly, if you open the Source Control view, then select, say, the explorer view, then close the explorer view, you expect the visible view to go back to the Source Control view but it sometimes shows the Debug view. I see this behavior on master and I don't think it is related to this PR.

Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

Tested on UBUNTU 18.04
Switch workspaces, open a multi-root workspace with multiple git project, I was able to change the SCM for each GIT project, the "AMEND" button and "COMMIT" text field always present. Open + close workspace and the refresh always occurs.

Signed-off-by: Nigel Westbury <nigelipse@miegel.org>
@westbury westbury force-pushed the fix-missing-widgets branch from 1f27efa to f319804 Compare May 11, 2020 14:45
@akosyakov
Copy link
Member

akosyakov commented May 12, 2020

Not sure whether it is related to the PR, but the input is styled differently now:
Screenshot 2020-05-12 at 09 51 45
Before:
Screenshot 2020-05-12 at 09 51 49

some padding is missing

Should I file another issue for it?

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

i cannot reproduce missing input anymore.

@akosyakov
Copy link
Member

Filed #7794 for bogus input styling.

@westbury westbury merged commit 57f820c into master May 12, 2020
@westbury westbury deleted the fix-missing-widgets branch May 12, 2020 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scm issues related to the source control manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the scm commit input is gone
3 participants