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

C#: Redesign MSBuild panel #80260

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos commented Aug 4, 2023

4.1 stable This PR
Before Problems tab (List view)
Problems tab (Tree view)
Output tab

@YuriSizov
Copy link
Contributor

This can now be finished on top of #80227. Would love to have this in 4.2 🙂

@raulsntos raulsntos marked this pull request as ready for review September 26, 2023 02:18
@raulsntos raulsntos requested review from a team as code owners September 26, 2023 02:18
@raulsntos raulsntos force-pushed the dotnet/msbuild-panel branch 3 times, most recently from ec42a9f to c64981c Compare September 26, 2023 14:50
@YuriSizov
Copy link
Contributor

image

Is the first item supposed to be empty? I guess it has to do with the "Unknown file" thing, but perhaps can be handled a bit nicer, with some text?

hbTools2.AddChild(_toggleLayoutButton);

// Show Search.
_showSearchButton = new Button
Copy link
Contributor

@YuriSizov YuriSizov Sep 26, 2023

Choose a reason for hiding this comment

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

You need to replace Flat = true with theme type variation FlatButton, so it has a nice pressed state.

@YuriSizov
Copy link
Contributor

YuriSizov commented Sep 26, 2023

I also noticed that you have a couple of your own dialogs, including an error dialog. First of all, you could use EditorNode::show_warning, probably. If you can't or you want to have more controlled and complex dialogs that you create yourself, you need to create them with set_unparent_when_invisible(true) and use EditorInterface::popup_* methods to pop them up. Without that your dialogs cause "Transient parent has another exclusive child.".

image

(Also there is this strange error that I got from creating a new C# script in the editor, it throws trying to call callp for some reason. I haven't investigated it further, as I doubt it's related to this PR.)

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

These are pretty minor problems, overall it all seems very nice!

- Redesign panel to look closer to the look of other Godot panels such as Output and Debugger.
- Moved list of problems and output log to separate tabs instead of using a HSplit.
- Added Tree/List layouts to the problems tab.
- Added search box to filter problems tab.
- Added `FileTree` icon, made from `FileList`. Both are used for the button that toggles the Tree/List layouts.
@raulsntos
Copy link
Member Author

I tried using set_unparent_when_invisible(true) and EditorInterface::popup_* but the dialog disappears immediately and the next time there's an error ERROR: Attempting to parent and popup a dialog that already has a parent. In any case it wouldn't be related to this PR, we've been using those dialogs for a while.

@akien-mga akien-mga merged commit c7a5a28 into godotengine:master Sep 27, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@raulsntos raulsntos deleted the dotnet/msbuild-panel branch September 27, 2023 10:41
@YuriSizov
Copy link
Contributor

I tried using set_unparent_when_invisible(true) and EditorInterface::popup_* but the dialog disappears immediately and the next time there's an error ERROR: Attempting to parent and popup a dialog that already has a parent.

That's weird. Make sure you don't parent it manually to anything, the popup methods handles that.

@raulsntos
Copy link
Member Author

raulsntos commented Oct 2, 2023

@YuriSizov It seems it's because of ProgressDialog::hide. The ProgressDialog::end_task is executed after we open the build error dialog. Maybe we should end the task before we show the error dialog, but also is this behavior expected? I mean if hiding ProgressDialog should also hide other dialogs.

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

Successfully merging this pull request may close these issues.

3 participants