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.

Sign up for GitHub

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

Jump to bottom

[3.2] C#: Re-work solution build output panel #42547

Merged
merged 1 commit into from
Oct 20, 2020
Merged

[3.2] C#: Re-work solution build output panel #42547

merged 1 commit into from
Oct 20, 2020

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Oct 4, 2020 •
edited by aaronfranke
Loading

Screenshot 1

  • The bottom panel button was renamed from Mono to MSBuild and now display an error/warning icon if the last build had issues.
  • Removed item list that displayed multiple build configurations launched. Now we only display the last build that was launched.
  • Display build output next to the issues list. Its visibility can be toggled off/on. This build output is obtained from the MSBuild process rather than the MSBuild logger. As such it displays some MSBuild fatal errors that previously couldn't be displayed.
  • Added a context menu to the issues list with the option to copy the issue text.
    Screenshot 2
  • Replaced the 'Build Project' button in the panel with a popup menu with the options:
    • Build Solution
    • Rebuild Solution
    • Clean Solution
      Screenshot 3

@neikeq neikeq added this to the 3.2 milestone Oct 4, 2020
@neikeq
Copy link
Contributor Author

neikeq commented Oct 4, 2020

This PR is for the 3.2 branch. I'll be opening one for master/4.0 soon.

@aaronfranke
Copy link
Member

The bottom panel button was renamed from Mono to .NET

Why make this change now? I thought we were still using Mono?

@neikeq
Copy link
Contributor Author

neikeq commented Oct 4, 2020

The bottom panel button was renamed from Mono to .NET

Why make this change now? I thought we were still using Mono?

We're still using the Mono runtime. But it doesn't make much sense to use the name of the runtime (Mono or CoreCLR) here. .NET is the term most developers are familiar with.

@aaronfranke
Copy link
Member

I would argue that Godot developers are extremely familiar with "Mono", since we have the "Mono module" and on the download page it's called "Mono version (C# support)". I think labeling things as .NET right now would cause confusion with people thinking that Godot is either using .NET Framework or .NET Core when it's not.

@akien-mga
Copy link
Member

The new "Build" popup menu doesn't really stand out as being an interactive menu, maybe it can be styled differently? (@Calinou @YeldhamDev)

Same for the other one on the same line, which also seem to have a toggled state (blue font) but it's not super obvious IMO that they're toggleable. UX advice welcome (also ping @groud).

@YeldhamDev
Copy link
Member

Setting flat to off would likely help indicate that those are buttons. Either that or icons.

@groud
Copy link
Member

groud commented Oct 6, 2020

UX advice welcome (also ping @groud).

This is what I would do personally:

  • Move the actions outside of the menu. We can have three buttons there, there is no need to hide 3 actions when we have plenty of space. Eventually move the buttons to the bottom, so that it looks less like a title.
  • Use icons for each action, or, as @YeldhamDev suggested, make the buttons not flat.
  • I am not sure what the Warning/Errors buttons are for. If those are used to toggle the visibility, I am not sure it is very useful. We don't have it in the debugger, so I would keep things consistent there.
  • If there is only one tab, using tabs is not very useful. So I'd remove the tab.

This is what I would do, but he current interface is already quite good. It can be merged in the current state too.

@aaronfranke
Copy link
Member

Also, if there is only one tab, maybe instead of "Mono" or ".NET" this menu should be "MSBuild"?

@neikeq
Copy link
Contributor Author

neikeq commented Oct 6, 2020

The new "Build" popup menu doesn't really stand out as being an interactive menu, maybe it can be styled differently?

I thought so too, but this seems to be the look of menu buttons. It's the same for the text editor for example, but it's specially confusing in this case with only one button.

  • Move the actions outside of the menu. We can have three buttons there, there is no need to hide 3 actions when we have plenty of space.

I think that would look bad unless they were icon-only buttons (and I can't find any icons for them). It also is an overload of information as it's not very common to rebuild or clean the solution.

  • I am not sure what the Warning/Errors buttons are for. If those are used to toggle the visibility, I am not sure it is very useful. We don't have it in the debugger, so I would keep things consistent there.

Pretty much every editor/IDE has this option, and I myself found it useful many times when there are lots of warnings. That said I will make them use icons as suggested by @YeldhamDev.
I'm not sure about them being flat. Are toggle buttons in the Godot editor generally flat or non-flat?

  • If there is only one tab, using tabs is not very useful. So I'd remove the tab.

Initially the plan was to add other tabs later on, but the only ones I have in mind now are not for sure and may even be better somewhere else. So yeah, I suppose it's better to remove them and go with the "MSBuild" name for the bottom panel as @aaronfranke suggested.

@neikeq
[3.2] C#: Re-work solution build output panel
fdfba05 
- Removed item list that displayed multiple build
  configurations launched. Now we only display
  the last build that was launched.
- Display build output next to the issues list.
  Its visibility can be toggled off/on.
  This build output is obtained from the MSBuild
  process rather than the MSBuild logger. As such
  it displays some MSBuild fatal errors that
  previously couldn't be displayed.
- Added a context menu to the issues list with
  the option to copy the issue text.
- Replaced the 'Build Project' button in the panel
  with a popup menu with the options:
  - Build Solution
  - Rebuild Solution
  - Clean Solution
- The bottom panel button was renamed from 'Mono'
  to '.NET' and now display an error/warning icon
  if the last build had issues.
@neikeq neikeq force-pushed the 3.2-rework-csharp-build-panel branch from 5bf8078 to fdfba05 Compare October 11, 2020 14:52
@neikeq
Copy link
Contributor Author

neikeq commented Oct 11, 2020

Updated. Did everything I mentioned in my last comment. Additionally:

  • Added an icon to the Build menu button to make it more clear it's not a label.
  • Made the Show Warnings, Show Errors and Show Output buttons non-flat to make it more clear they're not labels.
  • Removed spacers. All those buttons are aligned to the left now.

Let me know what you think. (You can compare before and after in the top comment edition history).

@YeldhamDev
Copy link
Member

One more recommendation: Move the "Show Output" button to the right, together with the side that the output actually appears.

@akien-mga
Copy link
Member

Let's merge to give it some testing in the upcoming 3.2.4 beta 1, and we can improve from there.

@akien-mga akien-mga merged commit 0237d42 into godotengine:3.2 Oct 20, 2020
@akien-mga
Copy link
Member

Thanks!

@neikeq neikeq mentioned this pull request Oct 23, 2020
Merged
@akien-mga akien-mga modified the milestones: 3.2, 3.3 Apr 21, 2021
@neikeq neikeq deleted the 3.2-rework-csharp-build-panel branch May 13, 2021 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
enhancement topic:dotnet topic:editor
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.
5 participants
@neikeq @aaronfranke @akien-mga @YeldhamDev @groud