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

View Mod Contents First Draft & TreeDataGrid Improvements #1023

Merged
merged 77 commits into from
Mar 12, 2024

Conversation

Sewer56
Copy link
Member

@Sewer56 Sewer56 commented Mar 5, 2024

fixes #989

Implementation includes:

  • Dynamically changing icons on open/close.
  • Custom theming for row headers and contents.
  • Sorting, by name and size.
    • Including putting folders first, before files, to match common file explorer behaviour.
  • Designer previews for everything.
  • Backend code to support all of this.
  • View correctly restores after app reboot.
  • Asynchronous view loading.
  • Cached ViewModels.
  • A new 'Loading' and 'Error' view/panel.
  • LoadoutGrid buttons are now disabled if unselected.
  • Slightly improved LoadoutSynchronizer perf (side effect of making a change to AGamePathTree).
  • App no longer displays trace logging in release builds (improves perf on small DB reads by ~70%).
  • Subscribing to Tree Node VM in DynamicData is now lazy, improving perf to show a view.

Additional changes:

  • Existing AdvancedInstaller dialogs now use a new TreeDataGrid helper to avoid repeated code.
  • Fixed broken Virtualization in some cases of TreeDataGrid.
  • TreeDataGrid columns can now be DI injected, instead of hardcoding them for each grid.
  • Removed 1px header hack from 'AdvancedInstaller' by avoiding unnecessary Grid use.
  • Removed unused x:Name declarations.
@Sewer56 Sewer56 requested a review from a team March 5, 2024 02:28
@Sewer56 Sewer56 self-assigned this Mar 5, 2024
@Sewer56 Sewer56 added Design UI/UX This is related to the UI. area-loadout-view labels Mar 5, 2024
@Sewer56 Sewer56 marked this pull request as ready for review March 5, 2024 02:28
@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2024

Codecov Report

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

Project coverage is 55.30%. Comparing base (ec39298) to head (f992366).
Report is 20 commits behind head on main.

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1023      +/-   ##
==========================================
- Coverage   56.80%   55.30%   -1.50%     
==========================================
  Files         645      660      +15     
  Lines       21248    21888     +640     
  Branches     1656     1710      +54     
==========================================
+ Hits        12069    12105      +36     
- Misses       8783     9387     +604     
  Partials      396      396              
Flag Coverage Δ
Linux 54.62% <23.63%> (-1.47%) ⬇️
Windows 54.45% <23.63%> (-1.57%) ⬇️
clean_environment_tests 55.28% <23.63%> (-1.51%) ⬇️
macOS 54.04% <23.63%> (-1.44%) ⬇️

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

Files Coverage Δ
...izers/Transformer/FlattenedToLoadoutTransformer.cs 85.85% <100.00%> (ø)
....UI/Content/Left/ModContent/ModContentViewModel.cs 96.00% <100.00%> (+8.50%) ⬆️
...aller.UI/Content/Right/Preview/PreviewViewModel.cs 91.66% <100.00%> (+12.87%) ⬆️
...nt/Right/SelectLocation/SelectLocationViewModel.cs 92.94% <100.00%> (+4.26%) ⬆️
...xusMods.App.UI/Controls/UnifiedIcon/UnifiedIcon.cs 79.10% <100.00%> (-0.31%) ⬇️
src/NexusMods.App.UI/Initializers.cs 60.00% <ø> (ø)
...pp.UI/LeftMenu/Loadout/LoadoutLeftMenuViewModel.cs 0.00% <ø> (ø)
...NexusMods.App.UI/Pages/MyGames/MyGamesViewModel.cs 100.00% <ø> (ø)
src/NexusMods.App.UI/Services.cs 98.63% <100.00%> (+0.11%) ⬆️
src/NexusMods.App.UI/TypeFinder.cs 100.00% <100.00%> (ø)
... and 24 more

... and 18 files with indirect coverage changes

@erri120 erri120 changed the title Feature: View Mod Contents First Draft & TreeDataGrid Improvements Mar 5, 2024
Copy link
Contributor

@Patriot99 Patriot99 left a comment

Choose a reason for hiding this comment

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

See comments

src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
lub nie jest już już dostępny w oryginalnej lokalizacji.</value>
</data>
<data name="ViewModInfoPage_NotFound_Title" xml:space="preserve">
<value>Mod Nie Znaleziony</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

How about such an alternative?

Suggested change
<value>Mod Nie Znaleziony</value>
<value>Nie znaleziono moda</value>
src/NexusMods.App.UI/Resources/Language.pl.resx Outdated Show resolved Hide resolved
Sewer56 and others added 7 commits March 12, 2024 02:07
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
lub nie jest już już dostępny w oryginalnej lokalizacji.</value>
</data>
<data name="ViewModInfoPage_NotFound_Title" xml:space="preserve">
<value>Mod Nie Znaleziony</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

How about such an alternative?

Suggested change
<value>Mod Nie Znaleziony</value>
<value>Nie znaleziono moda</value>
Co-authored-by: Patriot99 <31535921+Patriot99@users.noreply.github.com>
@Al12rs
Copy link
Contributor

Al12rs commented Mar 12, 2024

I will ignore translations fixes for review since we can deal with them after this PR.

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

Moved stuff to #1048

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

A single leftover thing to fix and I think we can merge, all the rest should be handled in future PRs.
Any additional feedback (e.g. translations) can be added as an issue or items to #1048

public CurrentModInfoSection Section { get; set; }

[Reactive]
public IViewModelInterface SectionViewModel { get; set; } = default!;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a puppy died somewhere when you wrote this. Make this nullable or set it to the DummyLoadingViewModel.

Copy link
Contributor

@Al12rs Al12rs left a comment

Choose a reason for hiding this comment

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

Remaining issues will be handled in future PRs.

@Al12rs Al12rs merged commit 021e5a1 into main Mar 12, 2024
7 checks passed
@Al12rs Al12rs deleted the view-mod-contents branch March 12, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design UI/UX This is related to the UI.
5 participants