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

Possible issue with component classes, perhaps mixed with pseudo-classes #1879

Closed
davep opened this issue Feb 24, 2023 · 14 comments
Closed

Possible issue with component classes, perhaps mixed with pseudo-classes #1879

davep opened this issue Feb 24, 2023 · 14 comments
Assignees
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented Feb 24, 2023

Coming from a report on Discord, it looks like there may be an issue with mixing pseudo-classes and component classes. I've not dived too deeply into this yet, just managed to create an isolated example of misbehaviour, so I'm recording it here at least as a placeholder.

The following code aims to set up a grid of datatables, using CSS to have the focused table have a different cursor colour from an unfocused table:

from textual.app        import App, ComposeResult
from textual.containers import Grid
from textual.widgets    import Header, Footer, DataTable

class Focustest( App[ None ] ):

    CSS = """
    Grid {
        grid-size: 4
    }

    DataTable .datatable--cursor {
        background: #333;
    }

    DataTable:focus .datatable--cursor {
        background: #c0c;
    }
    """

    @property
    def data_table(self) -> DataTable:
        data_table = DataTable[str]()
        data_table.add_columns("Column 1", "Column 2", "Column 3")
        data_table.add_rows(
            [(str(n), str(n * 10), str(n * 100)) for n in range(10)]
        )
        return data_table

    def compose( self ) -> ComposeResult:
        yield Header()
        with Grid():
            for _ in range( 16 ):
                yield self.data_table
        yield Footer()

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

If you run it you'll notice that the cursor colour does change under some circumstances, but not in the way you'd expect.

Tested with 0.12.0 so far; when I get a moment I'll revert to 0.11.1 and see how it worked there.

@davep davep added bug Something isn't working Task labels Feb 24, 2023
davep added a commit to davep/textual-sandbox that referenced this issue Feb 24, 2023
@pkazmier
Copy link

pkazmier commented Mar 7, 2023

Your example code works correctly in 0.11.1. Something definitely broke with 0.12.

@darrenburns darrenburns self-assigned this Mar 7, 2023
@darrenburns
Copy link
Member

Note to self: take a look at notify_styles_updated

@darrenburns
Copy link
Member

Closing - this was fixed in 0.17.0 on March 29 2023.

@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@pkazmier
Copy link

@darrenburns I do not believe this is fixed. I just tested with the sample code that Dave posted.

@pkazmier
Copy link

Of course, now that I've said that, I can't seem to reproduce again for a screenshot to show you. Will keep trying later today.

@pkazmier
Copy link

Got some weird behavior happening where mousing over the header rows will leave Column 3 highlighted sometimes.

image

@pkazmier
Copy link

Just got it to happen again, not sure how yet:

image

@davep
Copy link
Contributor Author

davep commented Apr 12, 2023

Testing here with 0.19.1, and also with main, I'm not quite seeing the effects you are at the moment, but am seeing:

  • Couple of odd hover effects (created with @darrenburns and I looking at my scren)
  • Cell 0,0 in table 0 (the top left table) never seems to show the :focus colour; all others do.

@davep davep reopened this Apr 12, 2023
@pkazmier
Copy link

The Column 3 effect above is easily reproduced.

Position your mouse at left side of the screen hovering over Column 1 of a data table. Then simply move your mouse to the right edge of the screen. Each Column 3 your mouse passes over will remain highlighted.

@lucasmouilleron
Copy link

lucasmouilleron commented Apr 12, 2023

Dirty fix for the Cell 0,0 and focus issue mentionned by @davep :

def _render_cell(self, row_index: int, column_index: int, style: Style, width: int, cursor: bool = False, hover: bool = False):
    self._row_render_cache.clear()  # dirty fix
    self._line_cache.clear()  # dirty fix
    return super()._render_cell(row_index, column_index, style, width, cursor, hover)

@darrenburns
Copy link
Member

darrenburns commented Apr 17, 2023

I've tracked it down to this flow of events:

  1. DataTable is focussed.
  2. render_line is called, which calls get_component_style and returns the non-focussed (and therefore incorrect) component style. It's incorrect because it hasn't yet been updated. This incorrect value is cached, but the corresponding cache key has has_focus=True.
  3. App.on_idle updates the styles of the DataTable, including the component styles. Subsequent calls to get_component_style will therefore be fine, however, since we’re caching the result, our cache now contains an incorrect entry.
[14:52:10] DEBUG                                              screen.py:381
DataTable(pseudo_classes={'enabled', 'focus-within'}) was focused
[14:52:10] PRINT                                        _data_table.py:1628
retrieved cursor style = #ffdddd on #ff0000
[14:52:10] PRINT                                          stylesheet.py:534
nodes getting updated = {DataTable(pseudo_classes={'enabled', 'focus',
'focus-within'})}

@darrenburns
Copy link
Member

darrenburns commented May 17, 2023

I believe this should be fixed with #2304 (v0.20.0+)

@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Task
Projects
None yet
Development

No branches or pull requests

4 participants