Skip to content

Fixes menu frame and unit test on the #2483.#2542

Merged
tig merged 4 commits into
gui-cs:v2_developfrom
BDisp:v2_menu-frame-fix
Apr 14, 2023
Merged

Fixes menu frame and unit test on the #2483.#2542
tig merged 4 commits into
gui-cs:v2_developfrom
BDisp:v2_menu-frame-fix

Conversation

@BDisp
Copy link
Copy Markdown
Collaborator

@BDisp BDisp commented Apr 13, 2023

Fixes #_____ - Include a terse summary of the change or which issue is fixed.

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

@BDisp BDisp requested review from migueldeicaza and tig as code owners April 13, 2023 23:10
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 13, 2023

@tig I broken the border around the listviews on the UICatalog, but I think you'll discovering that fast.

Copy link
Copy Markdown
Member

@tig tig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should just wait to fix these unit tests until after more of the new architecture is implemented.

Ok?

Comment thread Terminal.Gui/View/View.cs Outdated
Comment thread Terminal.Gui/View/View.cs Outdated
Comment thread Terminal.Gui/View/View.cs Outdated
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 14, 2023

@tig this should happens for your test or is an issue?

view-experiment-issue

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 14, 2023

@tig On the ScrollView color is failing. Maybe not a big deal but I need to look in better. OnDrawContentComplete must run after the OnRenderLineCanvas. Otherwise all the work on the OnDrawContentComplete will be damaged by the OnRenderLineCanvas.

@BDisp BDisp mentioned this pull request Apr 14, 2023
8 tasks
@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 14, 2023

Same as #2533 (comment). The LineCanvas is bad clipped.

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 14, 2023

@tig here is the explanation why the unit test Clear_Window_Inside_ScrollView is failing. Since the clipping is intentional bounds to console bounds, the container inside the scroll view overlaps the scroll view bounds. So, the fail is justified and I will comment it to pass in the checks. Then, after you are done with all yours tests, don't forgot to enable it, please. Thanks.

imagem

@BDisp
Copy link
Copy Markdown
Collaborator Author

BDisp commented Apr 14, 2023

@tig the Driver.Clip must be restored before call the OnRenderLineCanvas and the OnDrawContentComplete must be called after the OnRenderLineCanvas, unless this was intentional done for testing.

@tig can you change the .editorconfig to set the tab = 2 and not force a space before a parentheses on methods as Method()? Thanks.

@tig
Copy link
Copy Markdown
Member

tig commented Apr 14, 2023

@tig can you change the .editorconfig to set the tab = 2 and not force a space before a parentheses on methods as Method()? Thanks.

Both are a Miguel/Xamarin thing. I don't think we should change it.

@tig tig merged commit 5ae8e98 into gui-cs:v2_develop Apr 14, 2023
@BDisp BDisp deleted the v2_menu-frame-fix branch April 14, 2023 20:29
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.

2 participants