-
Notifications
You must be signed in to change notification settings - Fork 700
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
Get rid of Toplevel
- Introduce Runnable
and Overlapped
instead
#2491
Comments
#2544 makes progress on this. |
Tiled
and Overlapped
1st class conceptsToplevel
and introduced LayoutStyle = Overlapped
I'm Closing And putting the discussion here to simplify things. There's no need for two issues:. From 2502This is all debatable, but here's my thinking:
The My proposal:
Making this happen requires the following to be done first:
But it seems doable, and will significantly simplify the programming model (and codebase). What do folks think? |
In this case a Application.Run(new Button("I'm Popeye")) |
Yep, A better example (from #1720). The code here: #3154 (this part): void ColorBtnClicked (Button btn)
{
var fgColor = blocksPB.ColorScheme.Normal.Foreground;
var bgColor = blocksPB.ColorScheme.Normal.Background;
var colorPicker = new ColorPicker {
Title = btn.Text.
};
if (btn.Text == "Foreground") {
colorPicker.SelectedColor = fgColor.ColorName;
} else {
colorPicker.SelectedColor = bgColor.ColorName;
}
var dialog = new Dialog {
Title = btn.Text
};
dialog.LayoutComplete += (sender, args) => {
// TODO: Replace with Dim.Auto
dialog.Bounds = new Rect (0, 0, colorPicker.Frame.Width, colorPicker.Frame.Height);
dialog.X = Pos.Center ();
dialog.Y = Pos.Center ();
};
dialog.Add (colorPicker);
colorPicker.ColorChanged += (s, e) => {
dialog.RequestStop ();
};
Application.Run (dialog);
if (btn.Text == "Foreground") {
ChangeColor (colorPicker.SelectedColor, bgColor.ColorName);
} else {
ChangeColor (fgColor.ColorName, colorPicker.SelectedColor);
}
colorPicker.Dispose ();
} Would be: void ColorBtnClicked (Button btn)
{
var fgColor = blocksPB.ColorScheme.Normal.Foreground;
var bgColor = blocksPB.ColorScheme.Normal.Background;
var colorPicker = new ColorPicker {
Title = btn.Text
};
if (btn.Text == "Foreground") {
colorPicker.SelectedColor = fgColor.ColorName;
} else {
colorPicker.SelectedColor = bgColor.ColorName;
}
Application.Run (colorPicker);
if (btn.Text == "Foreground") {
ChangeColor (colorPicker.SelectedColor, bgColor.ColorName);
} else {
ChangeColor (fgColor.ColorName, colorPicker.SelectedColor);
}
} |
I see. Well done. |
This should be a goal of the new More details of what we mean here: |
Toplevel
and introduced LayoutStyle = Overlapped
Toplevel
and introduce Overlapped
as a 1st-class concept
I've closed: As a dupe of this. |
Toplevel
and introduce Overlapped
as a 1st-class conceptToplevel
and introduce Overlapped
as a 1st-class concept
Toplevel
and introduce Overlapped
as a 1st-class conceptToplevel
- Introduce Runnable
and Overlapped
instead
I've got some catch-up to do, but the skimming I did just now gave me warm fuzzies. If I have any thoughts to add, I'll do so after I finish some work stuff I'm currently at a wait phase of - hopefully later tonight. |
Ugh. I'm shot. Brain is too fried to give it good attention tonight. Will take a good look tomorrow for sure, though, barring any emergencies. 😅 |
I like a lot of what I've read so far and do have some thoughts to add to the discussion. I have to run to an appointment, but I'll work on writing stuff up when I return. Quickly, I think the stack idea is the right spirit, but a tree is probably a better structure for what is effectively a DOM. I think modeling everything after that paradigm is the simplest thing for us to do and meshes well with a lot of built-in .net stuff and familiar concepts for a wide range of developers. I'll expand on that/refine it when I get back, though. In particular, Now, if you also wanted to maintain a stack internally, purely for rendering purposes and not in the public api surface, that could potentially have interesting value. |
OK, now that I've had a couple of reads over the initial post, I can refine my earlier musing and add others... I'm going to break it up into multiple comments, trying to at least sorta stick to one major point each, starting at the top of the "Assertions" section and working my way down. Application/The stack stuffAgain, I like the intent here. But we don't actually have to function quite that low-level, here, since we're already on top of .net, which provides simpler abstractions for us. Correct me if I'm off base, but my reading of that is that it's intended/aimed at how we carry out both dispatching and handling of "messages" (which I'll use here in place of the word "event" so that I can use "event" to mean a C#/.net event instead), such as user input messages, property change messages, etc. If that reading is essentially correct, then that's one big place we can lean on .net and events. We already do, to an extent, but it's not a consistently applied concept or implementation, at present (though so far V2 has made significant improvements there). A big hurdle and potential pain point for consumers, with how it is now, is that there's not really a well-defined flow or order of things, that can be relied upon universally. The stack or tree concept of course makes good sense in that regard, insofar as it provides a defined root from which we can start and search until something is handled (or not). The logical conclusion of that line of thinking winds up being that we should have an actual DOM for the UI. I think that high-level concept, in general, actually addresses a lot of pain points we have or would like to address, and for more than just message passing. What I mean by not having to operate low-level is that we have events and we have more flexible data structures than stack at our disposal. Yes, a DOM has a single root, but that's the only mandatory standalone element. Even something as simple as XML has a single root, but then is a tree from there on. Where I think we can improve/rework things to fix itKeeping the concept of nodes in the DOM very high level and generic is a necessity, there, and enabling easy access and navigation through the tree is a highly likely and very reasonable expectation from a consumer. Most frameworks expose it very simply, as the tree it is, with every node having a reference to its parent as well as a collection of references to children (also terms I really think we should adopt instead of how we say SuperView and SubView, which is the same concept but different for no apparent value-add). A good template to follow is probably the way XML works - an XmlNode can contain 0 to n XmlNodes, and the specifics of them are implementation details of the document type definition or schema - not a definition set by XML itself, which remains completely content-agnostic as long as the syntax is valid. The specifics of those data structures would of course be one of the first discussion points, and I'd suggest use of collections in Whatever the structure, care also has to be taken to properly manage the lifetime of nodes. For example, if a user disposes a node, the base Dispose implementation could be responsible for making sure the reference is removed from its parent and that all of that disposed node's children are also dealt with appropriately. That specific examples is a perhaps draconian and simple one, but the point is there are things we need to either handle or at least not fail catastrophically or indeterminately on, to help avoid us facilitating a consumer getting into a bad program state that shouldn't have been allowed to happen in the first place. Alrighty... Going to take a little break for a while or until tomorrow and then come back for the next part, since I don't want to venture too far into implementation at this point in the process. And of course if I misread you, then the above is obviously not directly a response to that bullet point, but I do believe the general concepts stand regardless. |
My intent is not to completely rearchitect how the TG View hierarchy works. The tree model based on What you're proposing all makes good sense and is smart. But not for now. What this PR is about is the following: When Miguel built gui-cs, he decided to have a View subclass called Toplevel that combined three concepts: Supported being "run", with a complete "run state". Effectively a mini application within the process started when Later, a bunch of other functionalities was added to TopLevel. The Most pronounced was the idea of multiple, non-modal, overlapped Toplevels. This was introduced by @BDisp in the form of Here are examples of code in Application and Toplevel that highlight how complex and fragile this stuff is. This code is at the CORE of TG and it's freaking scary to look at. I want to be clear that I'm criticizing the code here, not who wrote it. I am responsible for a bunch of this too (to retain API compat we had no choice but to continue to hack at it). This issue, and the PR's I've started for it are about cleaning this mess up. It is about decoupling "runnable views", "event dispatch at the top of the hierarchy", and "overlapped" from Toplevel which will result in Toplevel going away. You could (and I think you are) argue that TG's model for event dispatch and View hierarchy could be redesigned too. I don't disagree. But that is not what we're doing here. Make sense? |
Ok, phew. I was a bit concerned for exactly that reason, at this stage of the game for v2. 😅 I'm dealing with some server rebuilds at the moment, so this is just an ack. I'll read over this in detail when I'm done. |
…on-And-Focus Preps for and partially Fixes #2491 - Refactors `Toplevel` and `Application` Navigation
Did you mean for this to get closed with that PR? Re-opening in case not. Go ahead and close if you want. Might be a good idea to continue the conversation elsewhere, though? |
Yea. Lesson learned. Dont write "Partially Fixes #xxx". Write "Fixes (Partially) #xxx". |
Ha yeah. There are a bunch of "closing comments" that cause that kind of automatic action. I think I neglected to mention that when I made a side remark about the issue title convention we use, probably like a year ago. 😅🤷♂️ Fixes, closes, resolves, and I think a few other words in combination with an issue number will close automatically. Only in English, I believe, or at least last time I saw the docs for it I'm pretty sure that was part of it. You know: In case you want to use ancient Egyptian or something. 😝 |
BTW, your comments above regarding a "DOM" DID get me thinking differently and I wanted to acknowledge that. I still am very much against having v2 be where we add a DOM to Terminal.Gui. However, I now see clear steps we can take in that direction that will enable such a thing post-v2. Some notes I wrote on this yesterday, that I thought i'd share: Focus Chain & DOM ideasThe navigation/focus code in The design is fundamentally the same as in v1: The logic for tracking and updating the focus chain is based on recursion up and down the // From v1/early v2: Note the `force` param.
private void SetHasFocus (bool newHasFocus, View view, bool force = false)
// From #3627: Note the `traversingUp` param
private bool EnterFocus ([CanBeNull] View leavingView, bool traversingUp = false) The need for these "special-case trackers" is clear evidence of poor architecture. Both implementations work, and the #3627 version is far cleaner, but a better design could result in further simplification. For example, moving to a model where Focus Chain: A sequence or hierarchy of UI elements (Views) that determines the order in which keyboard focus is navigated within an application. This chain represents the potential paths that focus can take, ensuring that each element can be reached through keyboard navigation. Instead of using recursion, the Focus Chain can be implemented using lists or trees to maintain and update the focus state efficiently at the By using lists or trees, you can manage the focus state without the need for recursive traversal, making the navigation model more scalable and easier to maintain. This approach allows you to explicitly define the order and structure of focusable elements, providing greater control over the navigation flow. Now, the interesting thing about this, is it really starts to look like a DOM! Designing a DOM for UI library involves creating a structured representation of the UI elements and their relationships.
This is all well and good, however we are NOT going to fully transition to a DOM in v2. But we may start with Focus/Navigation (item 3 above). Would retain the existing external I do NOT plan on tackling this in #3627. Nor do I consider it needed to address THIS issue. I now have #3627 working well enough (with tons of new primitive unit tests) that the "old model" will work fine. I still have to fix a bunch of issues in built-in Views causing There's a small chance that fixing these remaining issues may force me to revisit this decision. But for now... |
Partially Adresses #2491. Refactors how `Focus` works
This PR is for a major rehaul of how Terminal.Gui deals with non-Tiled view arrangements and "runnable" views.
@BDisp, @tznind, @dodexahedron, and @migueldeicaza - PLEASE READ AND TURN ON YOUR BRAINSTORM MODE
We have a chance to get the design of how all this works in v2 right. We should approach this throwing away existing notions of how Terminal.Gui has always worked in these regards. This does not mean we WILL throw all those things away, but that we should not constrain our thinking at this point.
I've written the below in the spirit of generating debate and ideas. I'm highly motivated to start working on this as I've been noodling on it for years. But my thinking is still very far from clear.
We will need a new "Multiple overlapped, MDI-like views" example (see #3636).
Related Issues & PRs
Toplevel
andApplication
Navigation #3634Focus
works #3627Slider
: API for setting options is clunky #3735Adornments
#3406Background
In v2, we have the following layout abilities:
View.Arrangement
and code inBorder.cs
for determining how a view (viaBorder
) can be moved and sized. Right now onlyViewArrangement.Fixed
andViewArrangement.Movable
have any implementation.Application.Run(Toplevel)
whereModal == true
.Dialog
,Messagebox
, andWizard
are the prototypical examples. When run this way, there IS az-order
but it is highly-constrained: the modal view has a z-order of 1 and everything else is at 0.Application.Run(Toplevel)
is called. Each Runnable view where (Modal == false
) has it's ownRunState
and is, effectively, a self-contained "application".Application.Run()
non-preemptively dispatches events (screen, keyboard, mouse , Timers, and Idle handlers) to the associatedToplevel
. It is possible for such aToplevel
to create a thread and callApplication.Run(someotherToplevel)
on that thread, enabling pre-emptive user-interface multitasking (BackgroundWorkerCollection
does this).Modal = true
. There are a few Scenarios (BackgroundWorkerCollection
) that utilize this to provide multiple "windows" that have a z-ordering and are NOT tiled. This is enabled by a bunch of crufty code inToplevel
(mostly inToplevelOverlapped.cs
) and a few other places. While there is a psuedo-z-order provided byToplevel._toplevels
, which is aStack<Toplevel>
, the API is confused and bug-ridden (e.g. not ALLToplevels
are added to this stack, bothToplevel.Current
andToplevel.Top
exist...along with whatever_toplevels.TryPeek()
returns).Assertions (these are up for challenge/debate!)
Application
should havepublic Stack<IRunnable> Runnables
where user-input events go only toRunnables.TryPeek()
, but all get timer, idle, and screen events. This is effectively how the Win32 non-pre-emptive multitasking system works.IRunnable
,ViewArrangement.Overlapped
wheremodalView.Z > allOtherViews.Max (v = v.Z)
, and exclusive key/mouse input.Application.Run(someView)
.someView
can be Overlapped or tiled. There's really no reasonsubviewA.X = Pos.Bottom(someView)
orsomeView.Width = Dim.Width (subviewA)
can't work.IRunnable
ViewArrangement
- We should addViewArrangement.Overlapped
. This is already done in Partially Adresses #2491. Refactors howFocus
works #3627.Visible = false
is sufficient as a default behavior for a user action that "closes" an Overlapped view (e.g. the equivalent of Alt-F4 for Windows or the (coming soon) close button inBorder
). If an app really wants an Overlapped to beDisposed
it can subscribe toVisibleChanging/VisibleChanged
events (TBI).Command.QuitToplevel
, which should be renamedCommand.QuitRunnable
.TopLevel
for subview should be Application scoped. This is already done in Partially Adresses #2491. Refactors howFocus
works #3627.Keyboard Nav
See https://github.com/tig/Terminal.Gui/blob/8e70e2ae8faafab7cb8305ec6dbbd552c7bf3a43/docfx/docs/navigation.md
Requirements
Runnable
as 1st class concept, not tightly coupled withOverlapped
, with a well-designed, clean, API and implementation.Overlapped
as 1st class concept, not tightly coupled withRunnable
, with a well-designed, clean, API and implementation.View
should be able to be used withApplication.Run
'd without it having to be a subclass ofToplevel
MenuBar
andStatusBar
should not be coupled withToplevel
, which prevents non-Toplevel
-dervived views from hosting them. See RefactorMenuBar
andStatusBar
to be adornments #2488View
with the mouse and keyboard. Partially addressed in #RefactorMenuBar
andStatusBar
to be adornments #2488. Full Issue: Add mouse/keyboard resizing of views #2537Justification / Issues with current systems (very old text; ignore if you want)
Overlapped is partially supported in v1 using
Toplevel
views. However, the v1 design is lacking:Application.Run<view>
(need a separateRunState
). It is not clear why ANY VIEW can't be run this way, but it seems to be a limitation of the current implementation.Dialog
). As proven byWizard
, it is possible to build a View that works well both ways. But it's way too hard to do this today.Window
today. It should be possible to enable the moving and resizing of any View (e.g.View.CanMove = true
).MdiContainer
stuff is complex, perhaps overly so. It's also misnamed because Terminal.Gui doesn't support "documents" nor does it have a full "MDI" system like Windows (used to). It seems to represent features useful in overlapping Views, but it is super confusing on how this works, and the naming doesn't help. This all can be refactored to support specific scenarios and thus be simplified.LineCanvas
andTileView
combined with @tig's experiments show it could be done in a great way for both modal (overlapping) and tiled Views.View
class hierarchy supports this in a simple, understandable way. In a world with non-full-screen (where screen is defined as the visible terminal view) apps, the idea thatFrame
is "screen relative" is broken. Although we COULD just define "screen" as "the area that bounds the Terminal.GUI app."MdiContainer/Overlapped
stuff is complex, perhaps overly so. It's also misnamed as Terminal.Gui doesn't support "documents" nor does it have a full "MDI" system like Windows (used to). It seems to represent features useful in overlapping Views, but it is super confusing on how this works, and the naming doesn't help. This all can be refactored to support specific scenarios and thus be simplified.The text was updated successfully, but these errors were encountered: