KEMBAR78
Show filtered stats by pfongkye · Pull Request #107067 · microsoft/vscode · GitHub
Skip to content

Conversation

@pfongkye
Copy link
Contributor

This PR fixes #105866

Pascal Fong Kye added 2 commits September 19, 2020 17:10
Use fixed size when pane grows instead of flex to avoid flickering
@pfongkye pfongkye marked this pull request as ready for review September 19, 2020 15:56
@pfongkye
Copy link
Contributor Author

Hi @isidorn
I've created a PR. Let me know if there's anything missing or any changes to do.
Thanks.

@isidorn
Copy link
Collaborator

isidorn commented Sep 21, 2020

@pfongkye thanks a lot for this PR. I will review it in detail and test it out tomorrow

@isidorn isidorn added this to the September 2020 milestone Sep 21, 2020
@isidorn
Copy link
Collaborator

isidorn commented Sep 22, 2020

@pfongkye thanks for your PR, I tried it out and seems to work good. Here's my feedback:

  • You are bringing back layout code for the shrink and grow of the Repl Filter, however the repl filter does not properly shrink and grow - it does not work with your changes. What I suggest is to remove this code completly from this PR and if you would like to tackle the shrink / grow issue please create an additional PR and test it out.
  • So from the above this PR should only contain the Badge which shows the number of filtered items work
  • Once you filter for some term that is nonexistent the debug console will be empty but the badge will not show anything. I suggest to show "showing 0 of N". That will make it more clear. And I suggest to also change that in problems view to be consistent

Thanks!

@pfongkye
Copy link
Contributor Author

@isidorn
Thanks for the feedback.
I'll make the necessary changes and create another issue for the shrink/grow problem. I'll keep you updated.

@isidorn
Copy link
Collaborator

isidorn commented Sep 22, 2020

Great thanks. Just ping me once it is ready for review again.

@pfongkye pfongkye marked this pull request as draft September 22, 2020 11:16
@pfongkye pfongkye marked this pull request as ready for review September 22, 2020 13:15
@pfongkye
Copy link
Contributor Author

@isidorn friendly ping to let you know the PR is ready for review.
I created an issue for the layout: #107230

@isidorn
Copy link
Collaborator

isidorn commented Sep 22, 2020

Looks great! Merging in and I will push some polish commits on top of this work.
Thanks a lot for your work!

@isidorn isidorn merged commit 7f93f17 into microsoft:master Sep 22, 2020
@isidorn
Copy link
Collaborator

isidorn commented Sep 22, 2020

Pushed some minor improvements via a47bf98

@pfongkye pfongkye deleted the issue/#105866 branch September 22, 2020 20:33
@pfongkye
Copy link
Contributor Author

pfongkye commented Sep 22, 2020

Pushed some minor improvements via a47bf98

@isidorn I like the refactoring since it's cleaner now. However, I noticed that a scenario might have been left out.

When you pause the debugger and begin to type in the filter, the badge will be updated. However, if you hit continue on the debugger, the badge will not be updated, showing x of n will remain the same even though there are more lines appearing.
I used the following listener for this scenario: this._register(this.tree.onDidChangeContentHeight(() => this.refreshReplElements(false))); but maybe there is a better solution.

@pfongkye pfongkye restored the issue/#105866 branch September 22, 2020 20:57
@isidorn
Copy link
Collaborator

isidorn commented Sep 23, 2020

@pfongkye great catch. I have tackled this via 8e82678

@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 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.

Show number of filtered debug console items

2 participants