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 Markdown.goto_anchor not scrolling the heading into view #4583

Merged
merged 5 commits into from
Jun 2, 2024

Conversation

davep
Copy link
Contributor

@davep davep commented Jun 1, 2024

Markdown.goto_anchor used to scroll the heading into view, there is code in there to do this. As was pointed out on Discord today, it no longer does. This PR changes how the request to scroll is done and now does the scrolling again.

Useful code to test before and after:

from textual.app import App, ComposeResult
from textual.widgets import Markdown

class MarkdownGotoApp(App[None]):

    def compose(self) -> ComposeResult:
        yield Markdown(
            "\n\n".join(
                f"# Heading {n}\n\n" +
                ("XXXXX " * 1000)
                for n in range(100)
            )
        )

    def on_mount(self) -> None:
        if self.query_one(Markdown).goto_anchor("heading-50"):
            self.notify("Markdown thinks we found and went to the anchor")
        else:
            self.notify("The anchor could not be found", severity="error")


if __name__ == "__main__":
    MarkdownGotoApp().run()

davep added 4 commits June 1, 2024 18:40
This used to, but it seems to have broken some way along the way; perhaps
because of changes to how scrolling works?

The problem was observed on Discord and this would seem to fix the issue.
@davep davep self-assigned this Jun 1, 2024
@davep davep added the bug Something isn't working label Jun 1, 2024
@davep davep requested review from willmcgugan and darrenburns June 1, 2024 17:50
@davep
Copy link
Contributor Author

davep commented Jun 1, 2024

Aaaaaaand the tests failed because GitHub threw up while trying to set up a test environment and it seems I no longer have rights to rerun a test. ಠ_ಠ

davep added a commit to davep/textual-sandbox that referenced this pull request Jun 2, 2024
@willmcgugan
Copy link
Collaborator

Thanks

@willmcgugan willmcgugan merged commit 1681cca into Textualize:main Jun 2, 2024
20 checks passed
@davep davep deleted the fix-goto-anchor branch June 2, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants