View diagnostics #1082

erri120 · 2024-03-19T11:17:46Z

Allows users to view and interact with diagnostics. Part of #327.

Fixes DiagnosticWriter output is mangled #1083
Fixes Fix MissingSMAPIEmitter #1084

Stuff that needs to be done in other PRs:

Text wrapping in the list isn't working
The horizontal line isn't styled with the severity color
Filter by severity and show chips in a "top bar"
Left menu icon for diagnostics

Fix Nexus-Mods#1084. Fixes the length check in `MissingSMAPIEmitter` for the amount of SMAPI installations. Adds a break, if SMAPI isn't installed, then it definitely won't be enabled.

codecov-commenter · 2024-03-19T14:31:55Z

Codecov Report

Attention: Patch coverage is 25.72347% with 231 lines in your changes are missing coverage. Please review.

Project coverage is 54.37%. Comparing base (c769522) to head (9b1fa65).
Report is 2 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1082      +/-   ##
==========================================
- Coverage   54.73%   54.37%   -0.36%     
==========================================
  Files         669      672       +3     
  Lines       21973    22160     +187     
  Branches     1704     1710       +6     
==========================================
+ Hits        12026    12049      +23     
- Misses       9554     9717     +163     
- Partials      393      394       +1

Flag	Coverage Δ
Linux	`53.69% <25.72%> (-0.38%)`	⬇️
Windows	`53.64% <25.72%> (-0.30%)`	⬇️
clean_environment_tests	`54.35% <25.72%> (-0.36%)`	⬇️
macOS	`53.17% <25.72%> (-0.32%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...rc/Games/NexusMods.Games.StardewValley/Services.cs	`100.00% <100.00%> (ø)`
...cs/DiagnosticTemplateIncrementalSourceGenerator.cs	`84.61% <100.00%> (+0.30%)`	⬆️
...ostics/Details/DiagnosticDetailsDesignViewModel.cs	`0.00% <ø> (ø)`
...Pages/Diagnostics/Details/DiagnosticDetailsPage.cs	`27.27% <ø> (ø)`
...usMods.App.UI/Pages/LoadoutGrid/LoadoutGridPage.cs	`18.75% <ø> (ø)`
src/NexusMods.App.UI/Services.cs	`98.78% <100.00%> (+0.06%)`	⬆️
src/NexusMods.App.UI/TypeFinder.cs	`100.00% <100.00%> (ø)`
...Games/NexusMods.Games.StardewValley/Diagnostics.cs	`0.00% <0.00%> (ø)`
...ames.StardewValley/Emitters/MissingSMAPIEmitter.cs	`0.00% <0.00%> (ø)`
...rols/Diagnostics/DiagnosticEntryDesignViewModel.cs	`0.00% <0.00%> (ø)`
... and 7 more

... and 4 files with indirect coverage changes

src/NexusMods.App.UI/Pages/Diagnostics/List/IDiagnosticListViewModel.cs

Al12rs · 2024-03-19T15:46:18Z

src/NexusMods.App.UI/Services.cs

            .AddView<DiagnosticEntryView, IDiagnosticEntryViewModel>()
+            .AddViewModel<DiagnosticEntryViewModel, IDiagnosticEntryViewModel>()


Do we need to register DiagnosticEntryViewModel? It needs to be constructed from code as it takes a Diagnostic in the constructor, is there an other reason to register it?

This is just there to keep the services consistent. It'd look weird if we only register the View and not the View Model.

Al12rs · 2024-03-19T15:58:51Z

src/NexusMods.App.UI/Pages/Diagnostics/DiagnosticIcons.cs

+    public static readonly IconValue DiagnosticIcon1 = new AvaloniaPathIcon(Geometry.Parse(
+            "M9.07495 19.25C8.80828 19.25 8.56662 19.175 8.34995 19.025C8.13328 18.875 7.98328 18.6834 7.89995 18.45L5.47495 12.75H1.19995V11.25H6.52495L9.07495 17.275L13.75 5.60005C13.8333 5.36672 13.9833 5.17505 14.2 5.02505C14.4166 4.87505 14.6583 4.80005 14.925 4.80005C15.1916 4.80005 15.4333 4.87505 15.65 5.02505C15.8666 5.17505 16.0166 5.36672 16.1 5.60005L18.525 11.25H22.8V12.75H17.475L14.925 6.77505L10.25 18.45C10.1666 18.6834 10.0166 18.875 9.79995 19.025C9.58328 19.175 9.34162 19.25 9.07495 19.25Z"
+        )
+    );
+
+    // From Design System "Custom Icons" section on Figma
+    public static readonly IconValue DiagnosticIcon2 = new AvaloniaPathIcon(Geometry.Parse(
+            "M2 9.87439V6.87439C2 6.32439 2.19583 5.85356 2.5875 5.46189C2.97917 5.07022 3.45 4.87439 4 4.87439H20C20.55 4.87439 21.0208 5.07022 21.4125 5.46189C21.8042 5.85356 22 6.32439 22 6.87439V9.87439H20V6.87439H4V9.87439H2ZM4 20.8744C3.45 20.8744 2.97917 20.6786 2.5875 20.2869C2.19583 19.8952 2 19.4244 2 18.8744V15.8744H4V18.8744H20V15.8744H22V18.8744C22 19.4244 21.8042 19.8952 21.4125 20.2869C21.0208 20.6786 20.55 20.8744 20 20.8744H4ZM10 17.8744C10.1833 17.8744 10.3583 17.8286 10.525 17.7369C10.6917 17.6452 10.8167 17.5077 10.9 17.3244L14 11.1244L15.1 13.3244C15.1833 13.5077 15.3083 13.6452 15.475 13.7369C15.6417 13.8286 15.8167 13.8744 16 13.8744H22V11.8744H16.625L14.9 8.42439C14.8167 8.24106 14.6917 8.11189 14.525 8.03689C14.3583 7.96189 14.1833 7.92439 14 7.92439C13.8167 7.92439 13.6417 7.96189 13.475 8.03689C13.3083 8.11189 13.1833 8.24106 13.1 8.42439L10 14.6244L8.9 12.4244C8.81667 12.2411 8.69167 12.1036 8.525 12.0119C8.35833 11.9202 8.18333 11.8744 8 11.8744H2V13.8744H7.375L9.1 17.3244C9.18333 17.5077 9.30833 17.6452 9.475 17.7369C9.64167 17.8286 9.81667 17.8744 10 17.8744Z"


I'm not happy with this approach, we are duplicating the definition of the icons, which isn't great.
I think this is ok for this PR but we should think about a better approach for this.

I have thought about it over the weekend and I couldn't find many solutions that don't end with an untyped reference in the code to something that is in the Theme project.
Maybe we can have a chat about this

Ideally, we'd have static values in the code project and the themes project would just import those as resources, but I haven't gotten that to work yet.

The Icons should be defined in the Theme project, as those are likely to change with the theme.

Since the theme project imports everything else, the code won't be able to access those icons. One possible solution would be to use the styles for this and have the tabs set class names instead of icon values. The themes project could just style the tabs as usual using selectors.

You can use theme resources from code without importing the theme project, as long as the resources are added to the App.xaml or whatever.
It isn't ideal because you have to reference them by key which is just a string. You also need to have access to a View, either from a codebehind or by getting the mainwindow

Al12rs

I just have a question above, the rest is fine considering a lot of stuff is to be done later

erri120 added 8 commits March 19, 2024 11:31

Add diagnostic page icon

2ead655

Update loadout left menu

5e719d2

Move files

0846fbc

Boilerplate

5eb281c

Update left menu

9f31897

Create View Model

9219f4e

Fix

03fdd44

Populate view

5060074

erri120 added the status-dont-merge label Mar 19, 2024

erri120 added this to the v0.4 milestone Mar 19, 2024

erri120 self-assigned this Mar 19, 2024

erri120 added 8 commits March 19, 2024 12:18

Fix discovery item name

2fb1e45

Use SourceList

be80e0f

Use ItemsControl

2337ff7

Fix

9a8804d

Use diagnostic title as page title

c6d1a06

Wrap text in details

89befbb

Add icons

52f7661

Fix mangled writer output

e55d2a4

Fixes Nexus-Mods#1083

erri120 removed the status-dont-merge label Mar 19, 2024

erri120 changed the title ~~WIP: View diagnostics~~ Mar 19, 2024

erri120 marked this pull request as ready for review March 19, 2024 12:54

erri120 requested a review from a team March 19, 2024 12:54

erri120 added 5 commits March 19, 2024 14:20

Fix MissingSMAPIEmitter

6fdce04

Fix Nexus-Mods#1084. Fixes the length check in `MissingSMAPIEmitter` for the amount of SMAPI installations. Adds a break, if SMAPI isn't installed, then it definitely won't be enabled.

Fix

2a743ec

Style list view

121bcac

Fix spacing

11b419f

Publish expensive GetLoadoutDiagnostics observable

9b1fa65

erri120 mentioned this pull request Mar 19, 2024

Better IDiagnosticWriter API #1085

Merged

Al12rs reviewed Mar 19, 2024

View reviewed changes

src/NexusMods.App.UI/Pages/Diagnostics/List/IDiagnosticListViewModel.cs Show resolved Hide resolved

Al12rs reviewed Mar 19, 2024

View reviewed changes

Al12rs approved these changes Mar 19, 2024

View reviewed changes

erri120 merged commit a3c2f41 into Nexus-Mods:main Mar 20, 2024
7 checks passed

erri120 deleted the feat/327-view-diagnostics branch March 20, 2024 09:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

View diagnostics #1082

View diagnostics #1082

erri120 commented Mar 19, 2024 •

edited

Loading

codecov-commenter commented Mar 19, 2024

Al12rs Mar 19, 2024

erri120 Mar 20, 2024

Al12rs Mar 19, 2024

erri120 Mar 20, 2024

Al12rs Mar 20, 2024

erri120 Mar 20, 2024

Al12rs Mar 20, 2024

Al12rs left a comment

		.AddView<DiagnosticEntryView, IDiagnosticEntryViewModel>()
		.AddViewModel<DiagnosticEntryViewModel, IDiagnosticEntryViewModel>()

View diagnostics #1082

View diagnostics #1082

Conversation

erri120 commented Mar 19, 2024 • edited Loading

codecov-commenter commented Mar 19, 2024

Codecov Report

Al12rs Mar 19, 2024

Choose a reason for hiding this comment

erri120 Mar 20, 2024

Choose a reason for hiding this comment

Al12rs Mar 19, 2024

Choose a reason for hiding this comment

erri120 Mar 20, 2024

Choose a reason for hiding this comment

Al12rs Mar 20, 2024

Choose a reason for hiding this comment

erri120 Mar 20, 2024

Choose a reason for hiding this comment

Al12rs Mar 20, 2024

Choose a reason for hiding this comment

Al12rs left a comment

Choose a reason for hiding this comment

erri120 commented Mar 19, 2024 •

edited

Loading