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

Scroll to cell by ID based on hash fragment #13285

Merged
merged 15 commits into from
Nov 14, 2022

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Oct 23, 2022

References

Reference implementation for jupyter/nbformat#317.

Fixes a 4.0-only regression in jumping to headings which had CSS-escapeable characters.

Closes #12928.

Code changes

  • setFragment is now always called for notebook on route change, whether in full windowing mode or not
  • removes CSS.escape which was no longer correct and breaking jumping to headings with characters that would be escaped (it was useful back then when we were using querySelector, see Allow cross-file anchors with leading number #11517, but not since we migrated to manual searching for heading in Windowed (Virtual) notebook #12554)

User-facing changes

  • Links to #cell-id=my-cell-id will scroll to the cell with such an id.

Backwards-incompatible changes

Links to headings starting with cell id= will now need to be updated with heading= prefix (I don't think there are many headings like that used in the wild).

@krassowski krassowski added this to the 4.0.0 milestone Oct 23, 2022
@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

packages/notebook/src/widget.ts Fixed Show fixed Hide fixed
packages/notebook/src/widget.ts Fixed Show fixed Hide fixed
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @krassowski

Would you mind adding some information in the documentation about this?

@krassowski
Copy link
Member Author

Good point. Documentation added, ready for another round of review/merge.

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks a lot @krassowski

The cell properties side panel snapshot is not sized correctly when updated.

Lately we are having again side panel sizing flakiness 😢 I think this is linked to #13037 that changes the workspace restoration logic.

@krassowski
Copy link
Member Author

please update documentation snapshots

@fcollonval
Copy link
Member

Kicking the CI

@fcollonval fcollonval closed this Nov 8, 2022
@fcollonval fcollonval reopened this Nov 8, 2022
@fcollonval
Copy link
Member

Thanks @krassowski for making the test more robust.

I saw that there is a regression on galata/test/documentation/debugger.test.ts-snapshots/debugger-variables-documentation-linux.png

@krassowski
Copy link
Member Author

I did not find a quick way to improve that one, so just reverted the assets. The visual tests are passing now but js-services are failing - looks unrelated at first glance?

@fcollonval
Copy link
Member

looks unrelated at first glance?

I confirm this is not related.

Thanks Mike - merging now.

@fcollonval fcollonval merged commit 6ecb558 into jupyterlab:master Nov 14, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve cursor placement, display and consistency in documentation screenshots
2 participants