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

Fixes #3073: Adds Bar as a replacement for StatusBar & Menu #3076

Closed
wants to merge 1,194 commits into from

Conversation

tig
Copy link
Collaborator

@tig tig commented Dec 23, 2023

Fixes

Todos

Still very much a Proof of Concept/Prototype

  • Implement Shortcut view that holds a Key, KeyBindingScope, Title, and help Text.
    • Renders just like a MenuItem.
      • Proper spacing/alignment of help text.
    • Supports arbitrary Views in the CommandView (first) position. E.g. CheckBox
    • Works with KeyBindingScope.Application and KeyBindingScope.HotKey.
    • Supports mouse clicks to activate the shortcut
  • Implement Bar view that
    • Looks just like the old StatusBar (StatusBarStyle = true, Orientation.Horizontal)
    • Looks just like the old MenuBar (StatusBarStyle = false, Orientation.Horizontal)
    • Looks just like the old Menu (StatusBarStyle = false, Orientation.Vertcal)
  • Update UI Catalog to use the new Bar
  • Update to leverage `Dim.Auto``
  • Update to leverage Pos.Align
  • Replace Toplevel.StatusBar

DATQ7MI 1

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tig tig changed the title Fixes #3037: Add's Bar as a replacement for StatusBar Fixes #3073: Add's Bar as a replacement for StatusBar Dec 26, 2023
@tig tig changed the title Fixes #3073: Add's Bar as a replacement for StatusBar Fixes #3073: Adds Bar as a replacement for StatusBar Dec 26, 2023
@tig tig mentioned this pull request Jan 19, 2024
@tig tig mentioned this pull request Feb 18, 2024
26 tasks
@tig tig mentioned this pull request Apr 12, 2024
16 tasks
@dodexahedron
Copy link
Collaborator

Oh certainly, I get that it's a targeted attempt at improving things (and it does).

I intentionally spoke generically, because the concept I intended to convey is a general concept, but it was also a question, though not really properly posed as one, to be fair.

In general, any object should be responsible for its own behaviors, or else a repository pattern is the appropriate alternative (where those objects are managed by a "repository" object that is responsible for telling everything what to do). That's a fundamentally different pattern from how most of TG is designed, though, so that's a non-starter anyway.

The easiest and most appropriate comparisons I can make are to other UI frameworks, since they all kinda set the de facto standards and expectations for developers.1

So, I'll use WPF, WinForms, and JavaScript as usual.

In all of those, the best place to handle user interaction is as close to the element they are interacting with or that the action is relevant to as possible. The loosest is JS, where you can certainly attach handlers to the window or document if you want, but are strongly advised not to do so.

The question

What is the aim/purpose of KeyBindingScope?
I'll leave that question bare, so as not to influence the response, and expand to better answer your request once I have that context.

Footnotes

  1. While it's perfectly cool to be somewhat different/disruptive to conventions and such when we have a good case for doing so, being different for no solid reason is perhaps not so advisable.

@tig
Copy link
Collaborator Author

tig commented May 31, 2024

Did you read the keybindingscope docs and keyboard conceptual docs?

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 31, 2024

Did you read the keybindingscope docs and keyboard conceptual docs?

Yes, and I'll re-read it again later, to see if I parse it any differently than before, but that's not what I'm looking for.

What I was looking for was something more direct and distilled, and intentionally on-the-spot, for a pointed reason. Because, if it can't be related like that without referencing a conceptual doc, it may be an indication that it could stand to benefit from more design work.

It was basically just a clumsy use of the Socratic method to lead to what my thought is on it, with the potential for my thoughts to be supplemented by your input, as well.

The short version of my opinion on it is it's unnecessary, at least as implemented. The concept and the goal are great, but it seems a little bit heavy-handed and kinda unintuitive. Almost like a solution in search of a problem, but not really to that degree.

Of course, that's from my perspective as someone who didn't write it or have the benefit of sharing a mind with who did implement it. 😅

@dodexahedron
Copy link
Collaborator

dodexahedron commented May 31, 2024

Put another way, it seems like an over-definition, if that makes sense.

But maybe I'm not grokking something staring me in the face.

Which is the value of the back-and-forth, since I'm always a fan of RTFM regardless. :)

And if that's the case, then it would just come down to doc tweaks.

@tig
Copy link
Collaborator Author

tig commented May 31, 2024

It was basically just a clumsy use of the Socratic method to lead to what my thought is on it, with the potential for my thoughts to be supplemented by your input, as well.

totally fair. I was responding on my phone as I was getting in bed. So for me it was basically being lazy.

The short version of my opinion on it is it's unnecessary, at least as implemented. The concept and the goal are great, but it seems a little bit heavy-handed and kinda unintuitive. Almost like a solution in search of a problem, but not really to that degree.

  • Focused -."Bind this key to me so that when I have focus nobody but me sees the key, but the command is invoked on me when it's pressed". The default.'
  • HotKey - "Bind this key to me so that regardless of whether I have focus or not, as long as my superview heirchy has focus, I'll get the command invocation if pressed." - By default, automatcially bound to the hotkey and the Command.Hotkey command, which is, effectively the "Default command" for a view.
  • Application - "Bind this key to me so that I'll always get the command invocation if pressed." - Enables a view to register an application-wide shortcut key, like StatusBar could do in v1 in a very hacky way.

All three of these are very common, real, use-cases.

It leverages the brilliant KeyBinding and Command infrastructure @tznind and @BDisp invented and radically simplifies View development. It now is very rare that a View will need to override OnKey... or subscribe to KeyPress events internally. It also enables configurability.

@tig
Copy link
Collaborator Author

tig commented May 31, 2024

Still very confused about why this PR is not actually comparing to v2_develop.

image

@dodexahedron: PLEASE HELP!

@tig tig changed the title Fixes #3073: Adds Bar as a replacement for StatusBar Fixes #3073: Adds Bar as a replacement for StatusBar & Menu May 31, 2024
@dodexahedron
Copy link
Collaborator

totally fair. I was responding on my phone as I was getting in bed. So for me it was basically being lazy.

Ha I feel ya there. I am guilty of that plenty of times myself.
And I always read it the next day and cringe. 😅

  • Focused -."Bind this key to me so that when I have focus nobody but me sees the key, but the command is invoked on me when it's pressed". The default.'
  • HotKey - "Bind this key to me so that regardless of whether I have focus or not, as long as my superview heirchy has focus, I'll get the command invocation if pressed." - By default, automatcially bound to the hotkey and the Command.Hotkey command, which is, effectively the "Default command" for a view.
  • Application - "Bind this key to me so that I'll always get the command invocation if pressed." - Enables a view to register an application-wide shortcut key, like StatusBar could do in v1 in a very hacky way.

All three of these are very common, real, use-cases.

It leverages the brilliant KeyBinding and Command infrastructure @tznind and @BDisp invented and radically simplifies View development. It now is very rare that a View will need to override OnKey... or subscribe to KeyPress events internally. It also enables configurability.

Right. That model is similar to RoutedCommand in WPF.

I think I need to sleep on this a bit more, too. Right now, I'm still getting the sense of over-definition or at least what feels like an unxpected inversion of control.

Here's what I would say, though, for sake of progress:

Even if we end up coming to some consensus on a change to make to it, the work to do so will be the same then as it would be right now. So, in my opinion, this particular thing isn't really a blocker for this PR necessarily - just an item to keep in mind for potential refinement. Heck, maybe even for a future version.

Though I also do feel like maybe the name of the type/property is part of what is nagging at me, maybe? Perhaps something mentioning Behavior instead of or in addition to scope might be more expressive? I dunno. Like I said, I would need to give it more thought. But right now I'd rather put those cycles into other work. :)

@tig
Copy link
Collaborator Author

tig commented Jun 8, 2024

Closing. Replaced by #3531

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants