-
Notifications
You must be signed in to change notification settings - Fork 814
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
WiP: Dialog widget #4044
Draft
davep
wants to merge
44
commits into
Textualize:main
Choose a base branch
from
davep:dialog-widget
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
WiP: Dialog widget #4044
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While the intention is that it won't be exported, I think it's sensible to give it a "public" name so we can refer to it in styling guides, so something can then style: Dialog > Body { ... } and so on, without needing to use leading underscores, making it look like they shouldn't do that.
Some of this text is just reminders for the moment, and proper linking will need to be done too; but that can come once everything has settled down.
Far from having the final styles yet; but this gets me started.
I was inheriting from various containers to build up the key parts of the dialog; the problem with that is that it's pretty easy for a developer using this dialog to also have done some pretty simple app-level styling of a Vertical or a Horizontal, and then things would start to fall apart in ways they might not expect. So here I just inherit from Widget and style the width/height/layout as needed. It's not much extra work and it helps keep some isolation.
While I'm trying to get to the bottom of how best to size all the various parts of the dialog, let's make it easier to see what part of the display is responsible for what space.
I want this, but this managed to sneak in too early; so reverting. There's currently a problem of wanting to width/height: auto this, but cap it at 1fr within the max-width/height of its container. Textual can't handle that right now; so we either need to find a better way of pulling this off, or CSS might need a bit of a tweak to support this.
The plan isn't go actually go with this; but this sort of implements what I'm aiming for; so we'll riff on this for the moment.
Tidying up the way I calculate the ideal height for the body. As mentioned before, this isn't the ideal way to do things; it can give a flicker as the dialog is first shown; but it's an approximation of what I want at least.
I think I'll want to dial in the styling some more; but this is the core framework for this.
More to add to this, more to write, but this kicks off the basics for documenting the Dialog.
I think there might be some to add, still to be decided, but now's a good time to remove the placeholder in case that doesn't happen.
There's little point in someone putting one of these anywhere other than inside a Dialog; so blow up if they do.
This helps make testing for this easier; for one thing.
Just some of the exception testing for now.
While I would never encourage it, ever, this does allow for a dialog within a dialog (and, really, just don't!).
There are more comprehensive examples in the main documentation; there's little point in having a cut-down version in the code too.
Open
Keeping this on hold for the moment, as per in-person request last week (as we want to do some extra work on CSS in support of some of the requirements in here). |
Looks like a cool feature definitely will be used by me!! <3 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A dialog container/widget.
Work in progress.
The current version of this code uses a workaround for some needed functionality in CSS, to help calculate the ideal with/height of parts of the widget.