textual icon indicating copy to clipboard operation
textual copied to clipboard

scroll_end off by one

Open aaronst opened this issue 3 years ago • 2 comments
trafficstars

It looks like Widget.scroll_end() is off by one line. Here's an app to reproduce:

from textual.app import App, ComposeResult
from textual.widgets import DataTable, Footer


class Test(App):
    BINDINGS = [("a", "add_row", "Add a row")]
    DEFAULT_CSS = """
    DataTable {
        height: 5;
    }
    """

    def compose(self) -> ComposeResult:
        rows = (
            ("1", "one"),
            ("2", "two"),
            ("3", "three"),
            ("4", "four"),
            ("5", "five"),
        )

        self.table = DataTable()
        self.table.add_columns("#", "number")
        self.table.add_rows(rows)

        yield self.table
        yield Footer()

    def action_add_row(self) -> None:
        self.table.add_row("6", "six")
        self.table.scroll_end()


Test().run()

After adding the sixth row, the DataTable scrolls to the fifth row.

aaronst avatar Nov 18 '22 02:11 aaronst

Hello

I believe because action_add_row() is synchronous, the relevant dimensions won't have been updated by the time scroll_end() is called.

(Details: add_row() does some stuff then calls check_idle(), sending out a message which is eventually handled by DataTable.on_idle(), where the dimensions are updated.)

I'm not sure if there's a simple way to guarantee the behavior you want, but the following worked the few times I tested it:

    async def action_add_row(self) -> None:
        self.table.add_row("6", "six")
        await asyncio.sleep(0)
        self.table.scroll_end()

Some other methods, such as Widget.mount(), return an awaitable. It could be useful if DataTable.add_row() did too.

lxnn avatar Nov 19 '22 03:11 lxnn

Thanks @lxnn, you're right. Static.update is another one - I guess any method that could affect the dimensions?

aaronst avatar Nov 19 '22 08:11 aaronst

await asyncio.sleep(0) didn't work for me but await asyncio.sleep(0.01) did....

rstomallen avatar Dec 06 '22 01:12 rstomallen

Does the following fix it?

self.call_later(self.table.scroll_end)

willmcgugan avatar Dec 06 '22 07:12 willmcgugan

        self.console_outputs.append(json.dumps(results, indent=4))
        self.query_one("#code", Static).update(self.console_outputs)
        self.call_later(self.body.scroll_end(animate=False, duration=0.01, easing="linear"))

no 🙁


From: Will McGugan @.> Sent: Tuesday, December 6, 2022 2:50 AM To: Textualize/textual @.> Cc: Tom Allen @.>; Comment @.> Subject: [EXTERNAL]Re: [Textualize/textual] scroll_end off by one (Issue #1215)

Does the following fix it?

self.call_later(self.table.scroll_end)

— Reply to this email directly, view it on GitHubhttps://github.com/Textualize/textual/issues/1215#issuecomment-1338918116, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVAFQFVA7BTAZ34DZFGD5WLWL3V4FANCNFSM6AAAAAASEAMEUU. You are receiving this because you commented.Message ID: @.***>

rstomallen avatar Dec 06 '22 14:12 rstomallen

self.call_later(self.body.scroll_end(animate=False, duration=0.01, easing="linear"))

Note that what you've done there isn't the same as what Will suggested though. You're passing the result of scroll_end to call_later, whereas Will is passing the function scroll_end to call_later. call_later takes a callback function.

Try it how Will wrote it.

davep avatar Dec 06 '22 14:12 davep

Ah, thanks for the explanation, I changed it, thanks. Yes, it does work, albeit slowly. My end use case is a MUD/telnet client which will scroll a lot more lines, faster. I'll play around with using the call_later callback.

Thanks again!

tom


From: Dave Pearson @.> Sent: Tuesday, December 6, 2022 9:09 AM To: Textualize/textual @.> Cc: Tom Allen @.>; Comment @.> Subject: [EXTERNAL]Re: [Textualize/textual] scroll_end off by one (Issue #1215)

self.call_later(self.body.scroll_end(animate=False, duration=0.01, easing="linear"))

Note that what you've done there isn't the same as what will suggested though. You're passing the result of scroll_end to call_later, whereas well is passing the function scroll_end to call_later. call_laterhttps://textual.textualize.io/api/message_pump/#textual.message_pump.MessagePump.call_later takes a callback function.

Try it how Will wrote it.

— Reply to this email directly, view it on GitHubhttps://github.com/Textualize/textual/issues/1215#issuecomment-1339445570, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVAFQFTZ6IQOBSD4YE6AZ2LWL5CLDANCNFSM6AAAAAASEAMEUU. You are receiving this because you commented.Message ID: @.***>

rstomallen avatar Dec 06 '22 14:12 rstomallen

I ran into this as well. Here was my hacky work-around:

        log_message = Label(str(message))
        await activity_log.mount(log_message)
        activity_log.scroll_end()
        activity_log.scroll_down()

I tried other combinations of call_later or setting a short timer, but this was the only way I could get the container to immediately scroll to the bottom. Most other attempts would initially result in the container scrolling to the next-to-the-last row and then about 100-200ms later the container would "correct" itself and scroll to the actual bottom.

keathmilligan avatar Dec 30 '22 19:12 keathmilligan

I suspect that #1902 will have helped here.

davep avatar Mar 01 '23 21:03 davep

I suspect that #1902 will have helped here.

I believe so, too. The original code works without any changes (i.e., calling self.table.scroll_end directly) as does the alternative solution with self.call_later(self.table.scroll_end), so I am closing this issue.

rodrigogiraoserrao avatar Mar 21 '23 13:03 rodrigogiraoserrao

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

github-actions[bot] avatar Mar 21 '23 13:03 github-actions[bot]