Add textual dialogs #2092
Add textual dialogs #2092
Conversation
| super().__init__() | ||
| self.interface = impl.interface | ||
| self.impl = impl | ||
| self.titlebar = TitleBar(title) |
There was a problem hiding this comment.
I noticed that the standard title bar expands when you click on it, which isn't really useful. Was that the reason for replacing it, or was there something else?
There was a problem hiding this comment.
The primary reason for replacing it was that Header() can (AFAICT) only have the title of the app. That's great for the default title of a MainWindow, but useless for every other location where we might need a header (such as the titlebar of a dialog, or the titlebar or any other window - or changing the title of the main window. Removing the "thick" rendering was a bonus.
textual/src/toga_textual/window.py
Outdated
| class CloseIcon(TextualWidget): | ||
| DEFAULT_CSS = """ | ||
| CloseIcon { |
There was a problem hiding this comment.
In the standard Textual header, I don't think this was intended to be a close button, but an application icon, as it's in the place where the icon would be on Windows.
Now that I'm a Mac user, I thought it was a close button too, and spent some time trying to work out why it didn't do anything. To avoid this confusion, maybe we'd be better off just removing it.
There was a problem hiding this comment.
That's a fair point - it's a decoration, but not a functional one.
I guess the alternative would be to make it functional... turn it into a button that does the close action on the window...
mhsmith
left a comment
mhsmith
left a comment
There was a problem hiding this comment.
Looks fine to me, except that the X on the close button was disappearing when you pointed at it, in both the VSCode terminal and the macOS terminal. It looks like this is caused by the border: none property, so I've added that to the hover block as well.
2 participants
Add Info, Question, Confirm, Error, and Stack Trace dialogs for Textual.
Also corrects the 24/25 line aspect ratio calculation identified in this comment
PR Checklist: