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

Create a new Bar view to replace StatusBar (and eventually MenuBar) #3073

Closed
tig opened this issue Dec 23, 2023 · 17 comments
Closed

Create a new Bar view to replace StatusBar (and eventually MenuBar) #3073

tig opened this issue Dec 23, 2023 · 17 comments
Assignees
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) enhancement
Milestone

Comments

@tig
Copy link
Collaborator

tig commented Dec 23, 2023

Possible dupe of:

I want to refactor StatusBar. It's actually a quite simple control, but was written before...

  • View.Text
  • View.HotKey with killer ToString support
  • Frames (for StatusItem spacing/alignment)
  • Consistency in eventing (KeyBindings and no Action).
  • KeyBindingScope.Application

It should just be a View that is Width = Fill and Y = AnchorEnd by default.

The new StatusItem:

public class StatusItem : View {
    public override Text {
        get => $"KeyBindings.FirstOrDefault(b => b.Scope != KeyBindingScope.Focus).GetKeyFromCommands(Commands.Default));
}

... plus some stuff to auotmatically enable / disable Border.Thickness = new Thickness (0, 0, 1, 0) for the separator.

StatusBar.Add is an override that sets subview.X appropriately (1st gets X = 0, subsequent get X = Pos.View(prev); last gets Width = Dim.Fill()).

Originally posted by @tig in #3070 (comment)

@tig tig added enhancement design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) labels Dec 25, 2023
@tig tig changed the title Create a new StatusBar to replace old... Create a new Bar view to replace StatusBar (and eventually MenuBar) Dec 26, 2023
@tig tig added this to the V2 Beta milestone May 25, 2024
@tig tig self-assigned this May 25, 2024
@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

The user needs to create a format Title to be able to prefix the shortcut. I think this should be the responsibility of the StatusBar generating the formatted string when redrawing. The current advantage is that the user can create a custom string for the shortcut, such as "~^A~ Add" for Ctrl+A. But the user may make a mistake by creating a different customization of the Shortcut property. The problem with this behavior is that the Title property will also return the shortcut when the user intends to obtain only the string "Add".
My question is whether it would be better for the StatusBar to automatically generate the string when redrawing, joining the Shortcut and the Title, just like the MenuBar, or leave it as is?

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

@tig why the StatusBar _shortcutDelimiter is equal to '='? Thus the Ctrl+A will be Ctrl=C. This doesn't make sense, I think.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

It's been a while since I worked on this. So I'm not clear what you're talking about.

Are you commenting on this PR?

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

I know this is not related to the StatusBar but I would just like to thank you for making the right decision so that the Dialog behavior does not automatically resize when adding new views like before. In this scenario, the Label, the TextField referring to the shortcut and the Button to clear the shortcut are now not showing and the user must provide the correct dimension. The Dialog constructor should really only be responsible for automatically resizing the message and buttons. Any view added later must be manipulated by the user. The user may even want to add a scroll bar to navigate. Perhaps a unit test that ensures this behavior doesn't change again would be beneficial. Thank you for your good decision.

image

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

It's been a while since I worked on this. So I'm not clear what you're talking about.

Are you commenting on this PR?

I was actually referring to what currently exists in v2_develop and I don't understand why you chose the '=' delimiter for the StatusBar.
But from what I noticed in your PR it seems that you will want to additionally use the shortcut prefixed by '_', which I think is a great idea.

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

var dialog = new Dialog { Title = "Enter the menu details.", Buttons = [btnOk, btnCancel], Height = Dim.Auto (DimAutoStyle.Content, 17, Driver.Rows - 10) };

@tig I'm trying to set the Dialog's height to Dim.Auto with a minimum of 17 and a maximum of Driver.Rows - 10 in case the available height is greater, if it is maximized, for example. But Dialog never assumes a height greater than 17. Why?
I think that if the maximum value assumes a value that is lower than the minimum, it will have to be the minimum value to always be considered. The maximum value is set to Driver.Rows - 10. If Driver.Rows is 20, the maximum value would be 20-10=10. As the minimum value is 17, this should be the value to be assumed as height. If Driver.Rows is 10, then the maximum value would be 10-10=0, but it will always be the minimum value to be taken into consideration. If the height does not fit in the available space, it will be the user's responsibility to activate scrolling to view the content.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

I know this is not related to the StatusBar but I would just like to thank you for making the right decision so that the Dialog behavior does not automatically resize when adding new views like before. In this scenario, the Label, the TextField referring to the shortcut and the Button to clear the shortcut are now not showing and the user must provide the correct dimension. The Dialog constructor should really only be responsible for automatically resizing the message and buttons. Any view added later must be manipulated by the user. The user may even want to add a scroll bar to navigate. Perhaps a unit test that ensures this behavior doesn't change again would be beneficial. Thank you for your good decision.

image

Yeah, I went back and forth on this.

I don't think I added a good enough unit test though. So if you want to suggest one that would be great.'

Also, note there are some bugs in there still. To see what I mean, open the UI Catalog about box and then drag it around. You'll see it is the wrong size when first created. These are bugs in Dim.Auto (Content) I'm still working through.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

var dialog = new Dialog { Title = "Enter the menu details.", Buttons = [btnOk, btnCancel], Height = Dim.Auto (DimAutoStyle.Content, 17, Driver.Rows - 10) };

@tig I'm trying to set the Dialog's height to Dim.Auto with a minimum of 17 and a maximum of Driver.Rows - 10 in case the available height is greater, if it is maximized, for example. But Dialog never assumes a height greater than 17. Why? I think that if the maximum value assumes a value that is lower than the minimum, it will have to be the minimum value to always be considered. The maximum value is set to Driver.Rows - 10. If Driver.Rows is 20, the maximum value would be 20-10=10. As the minimum value is 17, this should be the value to be assumed as height. If Driver.Rows is 10, then the maximum value would be 10-10=0, but it will always be the minimum value to be taken into consideration. If the height does not fit in the available space, it will be the user's responsibility to activate scrolling to view the content.

First, please don't use Driver.Rows. All Views (including Modals) should be constrained to a SuperView eventually (when TopLevel goes away).

Second, I think you're coming across bugs in Dim.Auto that I'm still struggling with. There be dragons there.

It would be amazing if you could narrow down what you're seeing into a new DimAuto unit test and submit an issue for it.

Finally, I'm not sure spending time on the Dyamic StatusBar scenario is a good use of time right now given I plan on rewriting it as part of #3076

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

First, please don't use Driver.Rows. All Views (including Modals) should be constrained to a SuperView eventually (when TopLevel goes away).

I don't know why to restrict a modal view defined with Dim.Auto to always use the size available in the console. For example, if the user increases or decreases the size of the console, why not allow a modal view to automatically resize? Just because it doesn't contain a SuperView? If it is because of this and the view is using a size like Dim.Auto, then SetRelativeLayout (new (Driver.Cols, Driver.Rows)) can be called for resizing. An exception to always keeping the same size is when the modal view has a fixed size.

Second, I think you're coming across bugs in Dim.Auto that I'm still struggling with. There be dragons there.

I understand and praise your efforts and will always help in any way I can. I'm not criticizing, for God's sake.

It would be amazing if you could narrow down what you're seeing into a new DimAuto unit test and submit an issue for it.

I'll introduce a unit test in a new issue for this.

Finally, I'm not sure spending time on the Dyamic StatusBar scenario is a good use of time right now given I plan on rewriting it as part of #3076

I was just testing this on a new IListDataSource that I'm developing and these problems were preventing me from testing it. Therefore it is not a refactoring of the Dynamic StatusBar.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

First, please don't use Driver.Rows. All Views (including Modals) should be constrained to a SuperView eventually (when TopLevel goes away).

I don't know why to restrict a modal view defined with Dim.Auto to always use the size available in the console. For example, if the user increases or decreases the size of the console, why not allow a modal view to automatically resize? Just because it doesn't contain a SuperView? If it is because of this and the view is using a size like Dim.Auto, then SetRelativeLayout (new (Driver.Cols, Driver.Rows)) can be called for resizing. An exception to always keeping the same size is when the modal view has a fixed size.

I'm thinking ahead to when we support non-full screen apps.

If a Modal has no SuperView, then yes, the size of the console should be used.

However, if a Modal HAS a SuperView, it should be constrained by it.

I know we don't have a way of making Modals have SuperViews today.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

I understand and praise your efforts and will always help in any way I can. I'm not criticizing, for God's sake.

Dude, I was just pointing out that there are bugs in there and you're probably encountering them.

@tig
Copy link
Collaborator Author

tig commented May 28, 2024

I was just testing this on a new IListDataSource that I'm developing and these problems were preventing me from testing it. Therefore it is not a refactoring of the Dynamic StatusBar.

Cool!

@BDisp
Copy link
Collaborator

BDisp commented May 28, 2024

I'm thinking ahead to when we support non-full screen apps.

I've also been thinking a lot about how to handle non-full screen apps and one of the solutions is to restrict the redrawing to just the Modal area and leave the rest of the console area intact. The problem is resizing the console will affect existing content that does not belong to the Modal. But it could be a start.

If a Modal has no SuperView, then yes, the size of the console should be used.

It is the normal procedure if the Modal is not of fixed size.

However, if a Modal HAS a SuperView, it should be constrained by it.

I don't understand why a Modal has to have a SuperView. For me, a Modal is a view that overlays all other views, which cannot be accessed while the Modal is open. If a Modal opens another Modal, this one will override all the others, including the other Modals.
Can you give me an example of a case where you need to add a Modal to a view, please.

I know we don't have a way of making Modals have SuperViews today.

Because it shouldn't really be necessary, in my opinion.

@dodexahedron
Copy link
Collaborator

But have you tried doing that, @BDisp ?

The results can be.... Interesting....

Trying to draw to just part of the screen buffer, especially if we are using ANSI esc sequences, can result in all sorts of strange output, inconsistently, even within the same terminal emulator.

It's a big can of worms to open.

@dodexahedron
Copy link
Collaborator

As an example, I was using the serial console redirection on a high end server system, a few days ago.

It claims to be xterm-256color compatible, and I was accessing it via Windows Terminal Preview as the host, with PowerShell 7.5 preview 3 as the shell, and then ssh to the server's management interface.

The first screen draw was fractured and unusable.

Upon resizing the window, some of that got better.

But most stuff didn't get any better until the content at each specific coordinate was updated explicitly.

They, like that idea, were trying to only update the necessary parts of the screen buffer, since it's designed to work well even over a slow dial-up modem interface.

But all the variables in between, plus I'm sure subtle bugs in their code, all conspired together to make it an unpleasant experience.

We can likely do much better, but probably still should do things like explicitly blank the rest of the screen every few frames or something, to mitigate that kind of problem.

@BDisp
Copy link
Collaborator

BDisp commented May 29, 2024

I haven't tried it yet because I don't think I'll be able to deal with non-Terminal.Gui content when the console is resized. First, how to obtain this content. Second, if you can get it, how to write to the console with the same type of letters, colors, etc.. That's why I haven't tried it yet, but one day I will, but I already know that the original content will be all messed up when resizing the console.

@dodexahedron
Copy link
Collaborator

Yep. It's a whole mine field.

tig added a commit that referenced this issue Jun 20, 2024
Fixes #3073. Adds `Shortcut`, `Bar`, and replaces `StatusBar`
@tig tig closed this as completed Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) enhancement
Projects
Status: ✅ Done
Status: 🏗 Approved - In progress
Development

No branches or pull requests

3 participants