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

Showing a widget on a higher layer changes the layout on a lower layer #1358

Closed
vsajip opened this issue Dec 14, 2022 Discussed in #1321 · 8 comments · Fixed by #2157
Closed

Showing a widget on a higher layer changes the layout on a lower layer #1358

vsajip opened this issue Dec 14, 2022 Discussed in #1321 · 8 comments · Fixed by #2157
Assignees
Labels

Comments

@vsajip
Copy link

vsajip commented Dec 14, 2022

Discussed in #1321

Originally posted by vsajip December 5, 2022
In the following script (slightly modified from that in the discussion), I attempt to implement a dialog using layers.

from __future__ import annotations  # noqa

import logging

from rich.console import Console
from textual.app import App, ComposeResult
from textual.containers import Container, Horizontal, Vertical
from textual.widgets import Label, Footer, Header

logger = logging.getLogger(__name__)

console = Console()

class Dialog(Container):

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.content = Container(
            Label("Example App"),
            Label("A tool for doing something or other"),
            Label("Copyright (C) 2022 The Copyright Holders"),
            Label("https://www.example.com/"),
            id="dialog",
        )

    def compose(self) -> ComposeResult:
        yield self.content

DIALOG_HEIGHT = 11
DIALOG_WIDTH = 51

def get_dialog_position(dw, dh):
    x = (console.width - dw) // 2
    y = (console.height - dh) // 2
    return (x, y)

class TestApp(App):

    BINDINGS = [('ctrl+d', 'show_dialog', 'Show Dialog')]

    CSS = '''
        Dialog {
            margin-left: %s;
            margin-top: %s;
            layer: dialog;
            width: %s;
            height: %s;
        }
        Screen {
            layers: base dialog;
        }
        #pane1 {
            height: 100%%;
            width: 1fr;
            border-right: solid $secondary-background;
            overflow: hidden;
        }
        #pane2 {
            height: 100%%;
            width: 1fr;
            border-right: solid $secondary-background;
            overflow: hidden;
            background: darkgray;
            color: black;
        }
        #pane3 {
            height: 100%%;
            width: 1fr;
        }
        .hidden {
            display: none;
        }
        #dialog {
            content-align: center middle;
            border: solid brown;
            background: ansi_white;
        }
        #dialog Label {
            text-align: center;
            width: 1fr;
            margin-top: 1;
            color: ansi_red;
        }
    ''' % (get_dialog_position(DIALOG_WIDTH, DIALOG_HEIGHT) +
           (DIALOG_WIDTH, DIALOG_HEIGHT))

    def __init__(self):
        super().__init__()
        self.title = 'Test App'
        self.dialog_showing = False
        self.dialog = Dialog(classes='hidden')

    def action_show_dialog(self):
        if not self.dialog_showing:
            logger.debug(f'dialog showing: {self.dialog_showing!r:<5} size: {self.panes.size}')
            self.dialog.toggle_class('hidden')
            self.dialog_showing = True
            self.set_timer(0.1, lambda: logger.debug(f'dialog showing: {self.dialog_showing!r:<5} size: {self.panes.size}'))

    def hide_dialog(self):
        if self.dialog_showing:
            self.dialog.toggle_class('hidden')
            self.dialog_showing = False

    def on_key(self, event):
        self.hide_dialog()

    def compose(self) -> ComposeResult:
        yield Header()
        yield Footer()
        pane1 = Vertical(Label('Pane 1'), id='pane1')
        pane2 = Vertical(Label('Pane 2'),
                         Label('Press any key to dismiss the dialog'),
                         id='pane2')
        pane3 = Container(Label('Pane 3'), id='pane3')
        self.panes = Horizontal(pane1, pane2, pane3, id='panes')
        yield self.panes
        yield self.dialog

def main():
    logging.basicConfig(filename='dlgbug.log', filemode='w',
                        level=logging.DEBUG, format='%(message)s')
    app = TestApp()
    app.run()

if __name__ == '__main__':
    main()

When Ctrl+D is pressed to show the dialog, after the dialog is displayed, even though it's in its own layer, the layout of the base layer seems to change slightly (width decreases by two, it seems, because a vertical scrollbar appears for no obvious reason). When the dialog is hidden, the layout reverts to what it was before the dialog was displayed. The following should illustrate the problem:

dialog-oddity

Note: This only happens on some terminals, not on all. I can get it repeatably on Linux Mint 64-bit with Cinnamon desktop.

I log the sizes of the Horizontal container on the base layer just before and after the dialog is displayed. This is what I get in the log:

dialog showing: False size: Size(width=220, height=22)
dialog showing: True  size: Size(width=218, height=22)

On terminals which don't exhibit the problem, the width remains unchanged.

This is with Textual from the repo (0.6.0+).

@mitosch
Copy link

mitosch commented Dec 15, 2022

I faced the same behaviour when developing a drop-down with a layer. I could not solve it so far, but noticed something, which could help:

When you comment out the line yield Header(), the scrollbar will not appear. Maybe the problem has something to do with the header.

I solved my problem temporarily by putting the drop-down list into a children layer (not directly to the app), maybe this also helps in your case? Something like:

          yield Header()
          yield Footer()
          pane1 = Vertical(Label('Pane 1'), id='pane1')
          pane2 = Vertical(Label('Pane 2'),
                           Label('Press any key to dismiss the dialog'),
                           id='pane2')
          pane3 = Container(Label('Pane 3'), id='pane3')
          self.panes = Horizontal(pane1, pane2, pane3, id='panes')

          # new:
          yield Container(
                    self.panes,
                    self.dialog
          )

@vsajip
Copy link
Author

vsajip commented Dec 15, 2022

Thanks for the suggestion, I'll try it out.

@vsajip
Copy link
Author

vsajip commented Dec 15, 2022

Good find. That workaround is great, in the test app, at least!

davep added a commit to davep/unbored that referenced this issue Dec 16, 2022
See Textualize/textual#1358 for a similar issue.
I'm still undecided at the moment if this is a bug, or correct behaviour.

Feels like a bug, but the workaround feels like good practice too.
@willmcgugan
Copy link
Collaborator

This issue has something to do with the scroll_spacing value returned by _arrange.py

The example has a few red-herrings. You can reproduce it even without panes1, panes2 and panes3.

@davep
Copy link
Contributor

davep commented Feb 27, 2023

Trying to get a pretty minimal reproduction of what seems to be a problem and I've got it down to this:

from textual.app        import App, ComposeResult
from textual.containers import Vertical
from textual.widgets    import Header, Footer, Label
from textual.binding    import Binding

class Dialog( Vertical ):

    def compose( self ) -> ComposeResult:
        """Compose the child widgets."""
        yield Label( "This should not cause a scrollbar to appear" )

class DialogIssueApp( App[ None ] ):

    CSS = """
    Screen {
        layers: base dialog;
    }

    .hidden {
        display: none;
    }

    Dialog {
        align: center middle;
        border: round red;
        width: 50%;
        height: 50%;
        layer: dialog;
        offset: 50% 50%;
    }
    """

    BINDINGS = [
        Binding( "d", "dialog", "Toggle the dialog" ),
    ]

    def compose( self ) -> ComposeResult:
        yield Header()
        yield Vertical()
        yield Dialog( classes="hidden" )
        yield Footer()

    def action_dialog( self ) -> None:
        self.query_one( Dialog ).toggle_class( "hidden" )

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

I was going to dive into what might be causing this, but coincidentally #1891 is being worked on at the moment, which in turn is a reaction to #1888, all of which could be related. So I'm leaving this test code here and we'll revisit once the outcome of those other changes is known.

davep added a commit to davep/textual-sandbox that referenced this issue Feb 27, 2023
@willmcgugan
Copy link
Collaborator

Will need to confirm if this is still an issue.

@davep
Copy link
Contributor

davep commented Mar 14, 2023

As of 0.15.1 this is still an issue. If I run the code above, when the "dalog" pops up, a vertical scrollbar appears.

@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants