-
Notifications
You must be signed in to change notification settings - Fork 31
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
Update to latest v2 branch to 2.0.0-v2-develop.2168 #292
Conversation
….Designer.cs fiiles
Fixes Window being core 'moveable' This goes considerable way to fixing: gui-cs/Terminal.Gui#3650 But can consider making the Arrangement property user configureable in the same way that we do with Visible (see TestSettingVisible_False) where user can change the value but it does not manifest in the behavior of the view (only for code gen).
…nt.Fixed` so user can still design it. Fixes gui-cs/Terminal.Gui#3650
@tig / @BDisp the keyboard handling seems to have changed and is no longer behaving itself with regards to handled and order of execution. For example
Expected behavior:
Actual behavior:
Here is the relevant code: private void ListView_KeyPress(object? sender, Key key)
{
// if user types in some text change the focus to the text box to enable searching
var c = (char)key;
// backspace or letter/numbers
if (key == Key.Backspace || char.IsLetterOrDigit(c))
{
this.searchBox?.FocusFirst(TabBehavior.TabStop);
this.searchBox?.OnKeyDown(key);
key.Handled = true;
}
} Another example is when editing a property.
The screen flashes (property save change but then re opens again). Only way to use keyboard to finish selection is if you manually tab to the button and complete selection with 'space' on the Button. If you could tell me whether this is a bug in Terminal.Gui or something I am doing wrong that would be super appreciated. Relevant code is primarily in: |
Can ou provide a unit test for the TG? @tig is on the gui-cs/Terminal.Gui#3655 and maybe he can see what is going on. Thanks. |
Ok I have fixed by switching to invoking And have standardized on KeyDown for all event handlers. I think previously if you Handled=true on KeyDown you would not then get a KeyUp too. Now you still do. Either that or it was a side effect that it worked. Having all be KeyDown makes things better. |
You should invoke
Ideally, to simulate key events, you go through
KeyDown/KeyUp are decoupled at the |
Only failing test is now crashing in init step as described in gui-cs/Terminal.Gui#3665 |
When I run the two tests separately I get no failures. When I run them together by selecting the file in the Test Explorer, sometimes both fail, sometimes one fails. It must be some object that is not being properly disposed when the other test starts. Any idea what could be happening? Some method that is initializing Application but not disposing of it completely before starting the next test. |
The error is in Ah, now I see there is this attribute
Removing it fix problem. I guess that make sense we can only have 1 Application at once. |
Yes, TG is a singleton API. |
Designing the I've updated comments on PR as this is a deal breaker for merging this. I've raised worst case which is: gui-cs/Terminal.Gui#3667 But there is also this unrelated one
|
Can you test with the PR gui-cs/Terminal.Gui#3663 and see if it's sill failing please. Thanks. |
Not sure how to do that short of adding Terminal.Gui as a git submodule. I can wait and just assume that this will fix though - it looks about right and I notice that it does not always occur that it misbehaves. |
But are you calling |
I do call FindDeepestView in TGD but I think in this case it just a simple null reference because |
All views have adornments (Margin, Border and Padding). So, |
This test doesn't fail in the TG: [Fact]
public void MenuBar_Has_Adornments ()
{
var mb = new MenuBar ();
Assert.NotNull (mb.Margin);
Assert.NotNull (mb.Border);
Assert.NotNull (mb.Padding);
} |
Another explanation is you're using a |
…roblems than it solves now See gui-cs/Terminal.Gui#3667
@tig , @BDisp I have begun testing this, it needs to be stable and working to go into v2. Hopefully this will not throw up any show stoppers in Terminal.Gui library that would require bugfixes and hence bumping nuget version (and taking in everything else that's added since 2168 - with all the breaking changes and re-testing that that would incur). As a real world test I am updating nfirestore-cli which is my Firestore Tui that I wrote in v2. I have added checkboxes for all the issues I discover in OP above under |
Button, Checkbox, Label etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. All works well.
Current broken code from changes in v2 are:
StatusItem
is goneCheckBox.Checked
is nowCheckBox.State
QA Testing
As noted we should now target nuget package
https://www.nuget.org/packages/Terminal.Gui/2.0.0-prealpha.216
Blocker issues:
Currently only 12 failing tests and all are the result of:
Menus
causes unexpected Exception Terminal.Gui#3652