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

As agreed in #72 this PR will switch all view models to inherit ComponentBase #82

Merged
merged 7 commits into from Aug 20, 2019

Conversation

SimonGeering
Copy link
Contributor

All ViewModel classes have been changed to inherit ComponentBase.

  • Where ViewModels had dependencies injected via constructor injection this has been changed to use property injection with the [Inject] attribute being added to existing properties where possible.
  • This has also allowed consolidation of [Parameter] and [CascadingParameter] code which was delegating to ViewModel methods or properties as well as any overrides of ComponentBase
  • All code blocks have been removed from all razor files. This should mean that going forward all substantive code logic outside of data and event action bindings can be pulled up into ViewModels, allowing full coverage of scenarios in unit tests.
  • Any "cheer graffiti" from the code blocks in the razor files has been moved into the corresponding location in the ViewModel.

To further improve the logical layout of the solution I would propose co-locating the view models in the Pages folder and re-naming them to align with the corresponding razor files so ViewModels/AvailabilityViewModel.cs would become Pages/Availability.ViewModel.cs

This way the code and markup would be right next to each other when being edited without the developer having to hunt in another folder for the corresponding file. I have not included this in the PR as it would be best done in the Git repo to help preserve the git history of the file.

@SimonGeering
Copy link
Contributor Author

Looks like, for some reason, VS2019 did not clean or re-run tests. Running at the command line shows the failures but not in VS until I re-loaded. May also be a CodeRush issue, either way looking into failing tests and will amend to the PR in a mo.

@SimonGeering
Copy link
Contributor Author

All tests now running locally
image

@SimonGeering
Copy link
Contributor Author

@csharpfritz Looks like this is now failing as the CI/CD pipeline is still using preview 7 SDK as highlighted in the screenshot below:

image

On a related note, would you be interested in a PR to switch to YAML based multi-stage pipelines?

@csharpfritz
Copy link
Contributor

csharpfritz commented Aug 18, 2019 via email

@SimonGeering
Copy link
Contributor Author

It all looks happy now :-)

Let me know re your thoughts on a PR to switch to YAML based multi-stage pipelines?

Thanks,
Simon G

@csharpfritz csharpfritz merged commit bd4e0f8 into FritzAndFriends:dev Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants