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.

Sign up for GitHub

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

Jump to bottom

Backport PR #15386: Improve scrolling to heading #15565

Merged
merged 3 commits into from Dec 29, 2023
Merged

Backport PR #15386: Improve scrolling to heading #15565

merged 3 commits into from Dec 29, 2023

Conversation

krassowski
Copy link
Member

Backport PR #15386: Improve scrolling to heading

@fcollonval @krassowski
Backport PR jupyterlab#15386: Improve scrolling to heading
6d16765 
* Fix scrolling on active heading

* Add option to scroll heading to the top for notebook
For editor it is not currently possible easily

* Switch to command mode if we are jumping to an heading in md cell

* Add tests

* Add doc string to new attribute

* Rebase follow-up

* Fix linter

* Update Playwright Snapshots

* Revert incorrect updates

---------

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 98d1e06)
@krassowski krassowski added the bug label Dec 26, 2023
@krassowski krassowski added this to the 4.0.x milestone Dec 26, 2023
@jupyterlab-probot jupyterlab-probot
Copy link

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

@krassowski
Change the scrollHeadingToTop setting to avoid behaviour change in …
3fd8d58 
…patch release

and add a note about upcoming change in the next minor release
@krassowski
Copy link
Member Author

Check internationalization changes / check_i18n is failing because we are adding a setting with user-facing setting string; we normally do not do this in backports to patch releases. We can override this rule (which we did in the past) if there is an agreement that adding the setting is more useful than the inconvenience of users with non-English interfaces for the time when the translations are not updated). Thoughts?

usage2 failures are unrelated and tracked in #15570, visual tests failures are unrelated (a fix needs backporting).

@jupyterlab jupyterlab deleted a comment from github-actions bot Dec 27, 2023
trungleduc
trungleduc reviewed Dec 29, 2023
View reviewed changes
packages/notebook/src/toc.ts
}
};

const cell = heading.cellRef;
const cells = widget.content.widgets;
const idx = cells.indexOf(cell);
// Switch to command mode to avoid entering Markdown cell in edit mode
// if the document was in edit mode
if (cell.model.type == 'markdown' && widget.content.mode != 'command') {
Copy link
Member

Choose a reason for hiding this comment

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

no need ===?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it is needed here; the only case these would return different results would be if cell.model.type were new String('markdown') but it cannot be that since it is typed to be literal rather than object string.

@krassowski krassowski merged commit b9bc300 into jupyterlab:4.0.x Dec 29, 2023
75 of 80 checks passed
@krassowski krassowski mentioned this pull request Dec 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trungleduc trungleduc trungleduc left review comments

Assignees

@krassowski krassowski

Labels
bug pkg:codemirror pkg:notebook pkg:toc tag:Testing
Projects
None yet
Milestone
4.0.x
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@krassowski @trungleduc @fcollonval