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

Strange column width behaviour when updating DataTable on *Selected event #4470

Open
TomJGooding opened this issue May 1, 2024 · 3 comments

Comments

@TomJGooding
Copy link
Contributor

This issue was originally posted in discussion #4463 by @luked42

There seems to be some strange behaviour with column widths when trying to update a DataTable on a *Selected event.

Here's an example based on the original post. When updating the table with the binding, the column widths work as expected. But if you swap this to update the table when a row is selected, it seems the column width isn't updated until the next refresh (when a row is hovered/highlighted, etc).

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


class MainWindow(App):
    BINDINGS = [("space", "update_table")]

    def compose(self) -> ComposeResult:
        self.ideas = ["a"]
        yield DataTable()

    def on_mount(self) -> None:
        table = self.query_one(DataTable)
        table.add_columns(*("foo", "bar"))
        table.cursor_type = "row"

        self.redraw()

    # def on_data_table_row_selected(self, event: DataTable.RowSelected) -> None:
    def action_update_table(self) -> None:
        self.ideas.append(f"{self.ideas[-1] * 2}")
        self.redraw()

    def redraw(self) -> None:
        table = self.query_one(DataTable)
        table.clear()
        for data in self.ideas:
            table.add_row(*(data, "-"), key=str(data))


if __name__ == "__main__":
    app = MainWindow()
    app.run()
@TomJGooding TomJGooding changed the title Strange column width behaviour when updating DataTable on `*Selected* event Strange column width behaviour when updating DataTable on *Selected event May 1, 2024
Copy link

github-actions bot commented May 1, 2024

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

@TomJGooding
Copy link
Contributor Author

TomJGooding commented May 1, 2024

Just to note in case it is relevant, there are also a couple of existing issues relating to column widths:

@luked42
Copy link

luked42 commented May 16, 2024

I've had some time to look a bit into this. Still getting familiar with the Textual framework so apologies for any oversights here.

From what I'm seeing / intuiting

Flow of code

  1. Selector key is hit
  2. self._set_hover_cursor(False) is called from action_select_cursor _data_table.py:2580. This ultimately queues a refresh of the current region
  3. main.py handles the redraw, clearing the table and readding rows (including the newly added row)
  4. The refresh/render line from step 2 is called, the column width (set to the max required for all rows) has not yet been updated yet, so DataTable renders using the old column width. Important to note here that we get called to render the lines that existed previous to step 3 (in terms of indices)
  5. _on_idle _data_table.py:1706 is now called where we have need to update dimensions. in _update_dimensions _data_table.py:1275 the column width is updated, additionally the virtual_size is updated to include the new row
  6. Compositor now calls for the extra line that was just added to be rendered as well. The previous rows how however not updated in this cycle

Minor points to note

  • if self._set_hover_cursor is not called in step one then the first render never happens and the entire table is re-rendered after the _on_idle call i.e it works fine.
    • My gut feel on this is that we're triggering the whole "current region" to refresh by clearing and adding the rows, otherwise I would have /thought/ thiat self._set_hover_cursor would only cause the selected row to refresh
  • I'm not yet familiar with the purpose of self._set_hover_cursor
  • If I trigger _on_idle from the redraw function (in a hacky way) it also works correctly as we've updated the column width before the render the first set of rows. Note that even if I do this still only the original set of rows and triggered to be rendered in the first cylce.
  • The reason other key bindings work to redraw is that they don't call self._set_hover_cursor / don't cause that premature render

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants