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 scrolling #11681

Merged
merged 2 commits into from
May 8, 2020
Merged

Fix scrolling #11681

merged 2 commits into from
May 8, 2020

Conversation

rchiodo
Copy link

@rchiodo rchiodo commented May 7, 2020

For #11554

ContentPanel was scrolling on every update. We really only need to scroll when the cell vms change.

Also added the setting 'alwaysScrollOnNewCell' such that if a new cell is run, we will auto scroll to the bottom in the interactive window.

@rchiodo rchiodo self-assigned this May 7, 2020
public componentWillReceiveProps(prevProps: IContentPanelProps) {
// Scroll if we suddenly finished or updated a cell. This should happen on
// finish, updating output, etc.
if (!fastDeepEqual(prevProps.cellVMs, this.props.cellVMs)) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it just be output updates where we would want to scroll to the bottom? Or is that trying to get too specific?

Copy link
Author

Choose a reason for hiding this comment

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

Ah good catch. Expanding Collapsing input cause it to scroll too. We really only want output updates.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there is a lot of other stuff in cellVMs

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 8, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov-io
Copy link

codecov-io commented May 8, 2020

Codecov Report

Merging #11681 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11681      +/-   ##
==========================================
- Coverage   60.67%   60.66%   -0.01%     
==========================================
  Files         627      628       +1     
  Lines       33879    33892      +13     
  Branches     4765     4769       +4     
==========================================
+ Hits        20556    20562       +6     
- Misses      12331    12335       +4     
- Partials      992      995       +3     
Impacted Files Coverage Δ
src/client/common/types.ts 100.00% <ø> (ø)
src/datascience-ui/react-common/arePathsSame.ts 75.00% <0.00%> (-12.50%) ⬇️
src/client/common/utils/platform.ts 64.70% <0.00%> (-11.77%) ⬇️
src/client/linters/pydocstyle.ts 86.66% <0.00%> (-2.23%) ⬇️
src/client/datascience/debugLocationTracker.ts 76.56% <0.00%> (-1.57%) ⬇️
src/client/common/process/proc.ts 14.49% <0.00%> (-0.73%) ⬇️
...ication/diagnostics/checks/pythonPathDeprecated.ts 96.15% <0.00%> (-0.08%) ⬇️
src/client/datascience/kernel-launcher/types.ts 23.07% <0.00%> (ø)
...client/datascience/kernel-launcher/kernelFinder.ts 77.01% <0.00%> (ø)
src/client/datascience/jupyter/kernels/helpers.ts 87.50% <0.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b06a77...2e4ea9a. Read the comment docs.

@rchiodo rchiodo merged commit 226b8f8 into master May 8, 2020
@rchiodo rchiodo deleted the rchiodo/scroll_jumper branch May 8, 2020 01:16
rchiodo added a commit that referenced this pull request May 8, 2020
* Fix scrolling

* Review feedback - fix scrolling on expand/collapse
greazer added a commit that referenced this pull request May 8, 2020
* Fix scrolling (#11681)

* Fix scrolling

* Review feedback - fix scrolling on expand/collapse

* Update changelog

* Update package.json

Co-authored-by: Jim Griesmer <jimgries@microsoft.com>
karthiknadig added a commit that referenced this pull request May 19, 2020
* Revert vscode-extension-telemetry changes for the release (#11602) (#11656)

* Revert "Fix slashes in telemetry unit tests (#11572)"

This reverts commit 7431c9c.

* Revert "Use vscode-extension-telemetry for our exceptions & error telemetry (#11524)"

This reverts commit d5065e6.

* Remove from changelog

* Port storage fix to release branch (#11673)

* Fix storage not being used (#11649)

* Fix storage not being used

* Add disposable to storage so it won't write after shutdown

* Fix dirty title

* Hack to get tests to pass

* Another way to get run all to not interfere

* Update changelog

* Port scrolling fix to release (#11688)

* Fix scrolling (#11681)

* Fix scrolling

* Review feedback - fix scrolling on expand/collapse

* Update changelog

* Update package.json

Co-authored-by: Jim Griesmer <jimgries@microsoft.com>

* Cherry-pick pipenv changes and pythonpath prompt changes to release (#11700)

* Show the prompt again if user clicks on more info (#11664)

* Show the prompt again if user clicks on more info

* Review feedback

* Use Learn more as text for the link.

* Leave pipenv in a corner until the user decides to select an interpreter (#11654)

* add onSuggestion option
* Swap onActivation with onSuggestion
* Update unit tests
* Remove registration of IPipenvService
* Move didTriggerInterpreterSuggestions logic inside pipenv locator
* Fix existing unit tests
* Add new unit tests
* Replace typemoq any param with object
* Shorten the tests
* Fix warning
* Duplicate teardown

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Update extension version (#11730)

* Update extension version

* Update date in changelog.

* Update change log with additional notes. (#11764)

* Cherry picks and version updates for bug fix release (#11878)

* Do not execute shebang as an interpreter until user has clicked on the codelens enclosing the shebang (#11816)

* Do not execute shebang as an interpreter until user has clicked on the codelens enclosing the shebang

* Rename

* Oops

* Update src/test/providers/shebangCodeLenseProvider.unit.test.ts

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>

* Update version and change log for bugfix release

Co-authored-by: Kartik Raj <karraj@microsoft.com>

* Clean up news

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com>
Co-authored-by: Jim Griesmer <jimgries@microsoft.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 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.

4 participants