More UI coding conventions

[erri120]

• Adds more context to OAPH
• Adds a section about design view models

[Al12rs]

What if a Design VM has properties containing other VMs, that require passing stuff in the constructor, can I just instantiate the Design version of the child VMs instead since we are already in a design VM?

This would avoid having to create mock data for the non design version in the parent DesignVM, on top of having to create that mock data already in the child DesignVM

[erri120]

As stated above, the design view model should only set up data. The normal VM would then take that data and create other VMs.

[erri120]

You can check the FOMOD changes I recently did. I massively cleaned up the design VMs: https://github.com/Nexus-Mods/NexusMods.App/pull/738/files#diff-9867405e9fbd8549cf89ffca330cee4eef1c8a421dba0cb637e5e5b36f59d897

[Al12rs]

But then you end up in a situation where if a standard VM has a constructor with custom parameters, you need to fabricate mock version of that data twice, once in the DesignVM to allow for a constructor without arguments, and then again in any parent DesignVM that needs to instantiate the standard VM.

This would then call for getting "fancy" to generate that data to avoid ending up with replicated code.

Is there a specific reason why we should not instantiate child designVMs inside designVMs?

[erri120]

If the parent has a list of children, you can make a data generation method on the child and re-use it in the parent.

[erri120]

I want to avoid a case where the design view model and the normal view model do the same thing by instantiating child view models, but the difference is in the type. This was an issue with the previous FOMOD code, where I'd had duplicate code that did the same thing but with a different type.

[Al12rs]

But if the alternative is

// inside a DesignVM constructor
ChildVM = new ChildVM(ChildDesignVM.GenerateDummyData());

Is it really any better than just:

// inside a DesignVM constructor
ChildVM = new ChildDesignVM();

Assuming ChildDesignVM extends ChildVM.

[erri120]

In that scenario there isn't much difference. My concern mainly lies with collection of child view models, since you usually construct those by connecting to a SourceCache:

//MyViewModel:
_sourceCache
  .Connect()
  .Transform(data => new ChildViewModel(data))
  .BindTo(out _children);

If you'd want to create a MyDesignViewModel and you have a ChildDesignViewModel, then you should not copy this code and change the Transform call to create a ChildDesignViewModel instead of a ChildViewModel. The ideal solution would be to just create dummy data in the MyDesignViewModel and let the base class MyViewModel create instances of ChildViewModel using that dummy data, similar to how a ChildDesignViewModel uses dummy data to pass it to the base constructor of ChildViewModel.

[Al12rs]

I see, so the general idea is to reduce as much as possible the changes in logic between MyVM and MyDesignVM.
I guess the example you posted above then is a better one to illustrate the point the documentation is trying to make.
If possible the DesignVM should just generate dummy data and feed it to the base VM.

[erri120]

I'll update the docs to be more precise.

[codecov]

Codecov Report

Merging #753 (4d403f4) into main (b324e46) will increase coverage by 0.03%.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #753      +/-   ##
==========================================
+ Coverage   62.73%   62.76%   +0.03%     
==========================================
  Files         585      585              
  Lines       17422    17420       -2     
  Branches     1219     1219              
==========================================
+ Hits        10930    10934       +4     
+ Misses       6170     6169       -1     
+ Partials      322      317       -5     

Flag	Coverage Δ
Linux	61.91% <0.00%> (-0.06%)	⬇️
Windows	61.88% <0.00%> (-0.01%)	⬇️
clean_environment_tests	62.74% <0.00%> (+0.02%)	⬆️
network_tests	?

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

Files	Coverage Δ
src/NexusMods.App.UI/ReactiveUIExtensions.cs	61.22% <0.00%> (-4.00%)	⬇️

... and 3 files with indirect coverage changes