Textualize / textual

The lean application framework for Python. Build sophisticated user interfaces with a simple Python API. Run your apps in the terminal and a web browser.
https://textual.textualize.io/
MIT License
24.09k stars 741 forks source link

DataTable not moving cursor into view when move_cursor invoked before scrollbar is visible in other screen #4524

Open SkPhilipp opened 1 month ago

SkPhilipp commented 1 month ago

This is a bug report for DataTable.move_cursor. When invoking add_row then move_cursor, the cursor doesn't move into view under the following conditions;

The result looks like so, with the cursor being 1 line below out of view:

Screenshot 2024-05-19 at 17 21 51

The code to reproduce:

#!/usr/bin/env python
from textual.app import App, ComposeResult
from textual.screen import ModalScreen
from textual.widgets import DataTable, Footer

class ModalScreenEmpty(ModalScreen):
    BINDINGS = [
        ("escape", "modal_cancel", "Cancel"),
    ]

    def action_modal_cancel(self):
        self.dismiss(123)

class MyApp(App):
    BINDINGS = [
        ("q", "quit", "Quit"),
        ("d", "duplicate", "Duplicate, then hit Escape"),
    ]

    def compose(self) -> ComposeResult:
        yield DataTable()
        yield Footer()

    def on_mount(self) -> None:
        table = self.query_one(DataTable)
        table.add_column("name")
        for _ in range(5):
            table.add_row("Alice")
            table.add_row("Bob")

    def action_duplicate(self):
        table = self.query_one(DataTable)

        def callback(_: any):
            row_key = table.add_row("Eve")
            row_index = table.get_row_index(row_key)
            table.move_cursor(row=row_index)

        self.push_screen(ModalScreenEmpty(), callback)

app = MyApp()
app.run()

To reproduce the issue, hit D followed by Escape repeatedly until a line is added that is just outside of view.

Workarounds;

1) Including overflow-y: scroll; appears to move the cursor as expected:

    CSS = """
    DataTable {
        overflow-y: scroll;
    }
    """

2) Placing the move_cursor behind a call_after_refresh.

github-actions[bot] commented 1 month ago

We found the following entry in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

willmcgugan commented 2 weeks ago

I can't reproduce this. Can you test against the latest version?