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

ReactiveDemo hack around VS xaml designer issue with inherits from generic classes #1837

Merged
merged 2 commits into from Nov 14, 2018

Conversation

Projects
None yet
4 participants
@Nilox
Copy link
Contributor

Nilox commented Nov 12, 2018

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Update ReactiveDemo sample

What is the current behavior? (You can also link to an open issue here)
ReactiveDemo sample solution does not build. After adding missing dependencies, Views cannot be opened in designer.

What is the new behavior (if this is a feature change)?
In ReactiveDemo sample:
NugetDetailsView.xaml can be opened in Design view.
Fixes missing references to PresentationCore, PresentationFramework and WindowsBase.

What might this PR break?
Nothting. Just wonder now if missing dependencies are not actually missing. I mainly work in VS for Windows, but I don't know how the sample hs to be used to target e.g. Xamarin.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

@Nilox Nilox requested a review from reactiveui/core-team as a code owner Nov 12, 2018

@dnfclas

This comment has been minimized.

Copy link

dnfclas commented Nov 12, 2018

CLA assistant check
All CLA requirements met.

// In our MainWindow when we register the ListBox with the collection of
// NugetDetailsViewModels if no ItemTemplate has been declared it will search for
// a class derived off IViewFor<NugetDetailsViewModel> and show that for the item.
public partial class NugetDetailsView : ReactiveUserControl<NugetDetailsViewModel>

This comment has been minimized.

@glennawatson

glennawatson Nov 12, 2018

Contributor

Where's this NugetDetailsViewBase come from. Seems redundant and unneccessary and the old approach was better.

This comment has been minimized.

@Nilox

Nilox Nov 12, 2018

Contributor

Fact is that NugetDetailsView.xaml cannot be edited in view designer. There is an issue in VS that does not support editing controls directly inherited from a generic class, so I proposed using a non generic class in between.
public class NugetDetailsViewBase : ReactiveUserControl<NugetDetailsViewModel> { }

public partial class NugetDetailsView : NugetDetailsViewBase
{
public NugetDetailsView()
{...
Other than that, I agree that the class is redundant from a functional point of view. However it is a matter of taste which approach is better - for a beginner, a MVVM framework with a sample project in which "a View cannot be viewed" may not look very promising.

Nevertheless, I have removed the intermediate class as requested.

@glennawatson

This comment has been minimized.

Copy link
Contributor

glennawatson commented Nov 12, 2018

Hi @Nilox thanks for the contribution. Due to being part of the .NET Foundation we require every contributor to do legal paperwork. Also the Reference updates to the csproj make sense, the deriving of the control the way you did it is not necessarily the standard way to use the ReactiveUserControl so I would just remove that for the moment.

@glennawatson

This comment has been minimized.

Copy link
Contributor

glennawatson commented Nov 13, 2018

That designer is so flakey. Bain of a lot of people's existance.

@glennawatson glennawatson merged commit 0a8d8fb into reactiveui:master Nov 14, 2018

1 check passed

license/cla All CLA requirements met.
Details

@Nilox Nilox deleted the Nilox:demodesigner branch Nov 14, 2018

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