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 "view as scrollable element" not working #5680

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

seeM
Copy link
Contributor

@seeM seeM commented Dec 10, 2024

The issue was due to cell output metadata being set to undefined by the Positron notebook controllers, causing the scrollable metadata to not be set here:

if (vm.model.metadata) {
vm.model.metadata['scrollable'] = true;
vm.resetRenderer();
}

We now set metadata and also set outputType since it also makes life easier for the ipynb serializer:

switch (outputType) {

QA Notes

  1. Run a Python notebook cell with the following code:
  # code with error
def divide_numbers(numbers):
    result = []
    for num in numbers:
        result.append(10 / num)
    return result

numbers_to_divide = [2, 4, 0, 5, 7]

try:
    divide_numbers(numbers_to_divide)
except Exception as e:
    error_message = f"Error: {e}\n" * 50  # Repeat error message
    raise RuntimeError(error_message)
  1. Click "View as scrollable element" - it should work.
  2. Also try the "Notebook: Toggle scrollable element" command with the cell selected.

The issue was due to cell output metadata being set to `undefined` by the Positron notebook controllers, causing the `scrollable` metadata to not be set here: https://github.com/posit-dev/positron/blob/54d791da6b5e015ff03a596a848c8893cf90d3bd/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts#L766.
@seeM seeM requested a review from nstrayer December 10, 2024 12:10
Copy link
Contributor

@nstrayer nstrayer left a comment

Choose a reason for hiding this comment

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

Looks good and works for me.
Took me a while to figure out how adding the outputType field allowed scrollable to work but I get now that adding any metadata means that the metadata field is defined and can then me updated with new fields etc..

@seeM seeM merged commit 6c6f6c4 into main Dec 11, 2024
5 checks passed
@seeM seeM deleted the 5603-fix-view-as-scrollable-element branch December 11, 2024 18:36
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2024
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.

2 participants