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

Start working on updating to pre710 nuget package #289

Merged
merged 46 commits into from
Jun 15, 2024
Merged

Start working on updating to pre710 nuget package #289

merged 46 commits into from
Jun 15, 2024

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Mar 22, 2024

Updates Terminal.Gui v2 dependency to latest.

@tznind tznind changed the title Start working on updating to pre636 nuget package Start working on updating to pre710 nuget package Mar 31, 2024
@tznind
Copy link
Collaborator Author

tznind commented Mar 31, 2024

200 failing tests

  • Application.Top is now null
  • Exception 'Must set AutoSize to false before setting Width.'
  • Key tostring seems to have changed to always be upper
  • New dependency on System.Drawing
  • TabView changes
  • ColorScheme init only properties break current code generation

@BDisp
Copy link
Contributor

BDisp commented May 14, 2024

TestColorScheme_RoundTrip unit test fails because ColorScheme is initializing like this:

            this.button = new Terminal.Gui.Button();
            this.greyOnBlack = new Terminal.Gui.ColorScheme();
            this.greyOnBlack.Normal = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black);
            this.greyOnBlack.HotNormal = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black);
            this.greyOnBlack.Focus = new Terminal.Gui.Attribute(Terminal.Gui.Color.Black, Terminal.Gui.Color.DarkGray);
            this.greyOnBlack.HotFocus = new Terminal.Gui.Attribute(Terminal.Gui.Color.Black, Terminal.Gui.Color.DarkGray);
            this.greyOnBlack.Disabled = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black);

And must be initialized like this:

            this.button = new Terminal.Gui.Button();
            this.greyOnBlack = new Terminal.Gui.ColorScheme()
            {
                Normal = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black),
                HotNormal = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black),
                Focus = new Terminal.Gui.Attribute(Terminal.Gui.Color.Black, Terminal.Gui.Color.DarkGray),
                HotFocus = new Terminal.Gui.Attribute(Terminal.Gui.Color.Black, Terminal.Gui.Color.DarkGray),
                Disabled = new Terminal.Gui.Attribute(Terminal.Gui.Color.DarkGray, Terminal.Gui.Color.Black)
            };

@tznind
Copy link
Collaborator Author

tznind commented May 14, 2024

We cannot use initializer lists in code gen because it is not supported. Need to add the constructor call instead.

@tznind
Copy link
Collaborator Author

tznind commented May 14, 2024

@BDisp fixed ColorSchemeToCode, should now use the new constructor in 03a2876

215 tests to go 😢

@BDisp
Copy link
Contributor

BDisp commented May 14, 2024

In the NullDim_ActsLikeAbsoluteZero unit test change to this, because View.Dim is never null now

Assert.That( v.Width, Is.Not.Null, "As of v1.7.0 a new View started getting null for its Width, if this assert fails it means that behaviour was reverted and this test can be altered or suppressed" );

@BDisp
Copy link
Contributor

BDisp commented May 14, 2024

This did the trick.

            var fSide = posView.GetType().GetField("<side>P", BindingFlags.NonPublic | BindingFlags.Instance) ?? throw new Exception("PosView was missing expected field 'side'");
            var iSide = fSide.GetValue(posView)

@BDisp
Copy link
Contributor

BDisp commented May 14, 2024

See how I got the field name.

image

@BDisp
Copy link
Contributor

BDisp commented May 14, 2024

TabView isn't copying the Tab.DisplayText which now is a View and is TabRow subview.

@tznind
Copy link
Collaborator Author

tznind commented May 15, 2024

Nice detective work on "<side>P". I think I will hold off on that for now though as I have raised gui-cs/Terminal.Gui#3482 which will likely let us get rid of the reflection entirely in the pos/dim helpers.

@tznind
Copy link
Collaborator Author

tznind commented May 15, 2024

Ok think I got all the DisplayText bits fixed

@BDisp
Copy link
Contributor

BDisp commented May 16, 2024

You'll need to create a Auto item to the DimType enum, otherwise it will assume the null for the Width property. I don't know if it will have many changes when be merged.

I was thinking about this and came to the conclusion that using DimType and common functions is not the best option because Pos/Dim is changing radically and with many references that are not common. I think the best way is to use Pos/Dim itself in a switch block and manipulate the different calculations in the options, you can use some common functions but it is not possible in all cases.

@tznind
Copy link
Collaborator Author

tznind commented May 16, 2024

Yes, I agree. Previously these were internal classes and TGD did not have visibles attribute (hence the methods and enums).

Now it should be more possible to use is and query properties on them.

But I think it will have to wait till that refactoring is done in gui-cs/Terminal.Gui#3482.

@BDisp
Copy link
Contributor

BDisp commented May 16, 2024

Another problem I'm encountering is with resizing a view because on the sides and bottom, as long as there is a border, it will just move the view instead of resizing it. I think the TG should rethink this behavior. Movement should only be possible from the top and sides and at the bottom it should only allow resizing.

@tznind
Copy link
Collaborator Author

tznind commented May 16, 2024

Movement should only be possible from the top and sides and at the bottom it should only allow resizing.

This is how it is supposed to work. Bottom right few cells are for resizing, the rest are for moving. This allows a 1xN view to be resized (regardless of whether or not it has a frame).

I think there is a bug in the mouse handling code of TGD. It worked before in v1 and v2 before all the mouse changes. Possibly in the migration to the new mouse API something is not translating positions correctly.

This is probably why all the mouse click and drag tests are now failing

@BDisp
Copy link
Contributor

BDisp commented May 16, 2024

I got some improvements but not enough :-)

WindowsTerminal_d60zKPSjUF

@tznind
Copy link
Collaborator Author

tznind commented Jun 1, 2024

Only 1 failing test now.

I think it might be around screen/local coordinates of views. I need to do more manual testing to understand any gaps. I especially need to test multiple nested views that are not at origin of parent view to ensure coordinates are all resolving correctly.

Other outstanding issues:

  • Selecting colour (green) seems to stick to selected views borders sometimes
  • Cannot resize FrameView
  • Creating Dialog as the root view results in size very small e.g. 1x1

@tznind tznind marked this pull request as ready for review June 15, 2024 14:30
@tznind tznind merged commit 86c83f8 into v2 Jun 15, 2024
1 check passed
@tznind tznind deleted the pre636-support branch June 15, 2024 14:33
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

2 participants