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

feat: a more flexible default view locator #1282

Merged
merged 13 commits into from Feb 20, 2017

Conversation

Projects
None yet
5 participants
@kentcb
Copy link
Contributor

kentcb commented Feb 20, 2017

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

This PR rolls up a solution to #1014 and #1171 and it supersedes PRs #1171 and #1230. It ended up being much easier to write a bunch of tests and then rewrite DefaultViewLocator. The code had gotten needlessly convoluted, and the new code is intended to be much more readable.

Does this PR introduce a breaking change?

No.

Please check if the PR fulfills these requirements

  • The commit follows our guidelines: https://github.com/reactiveui/reactiveui#contribute
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Double-check tests against old implementation (to prove no regressions)
  • Do any further necessary refactoring to make the code even easier to read

@kentcb kentcb referenced this pull request Feb 20, 2017

Closed

test and rewrite default view locator #1278

4 of 5 tasks complete

@kentcb kentcb changed the title test and rewrite default view locator feat: a more flexible default view locator Feb 20, 2017

@kentcb kentcb added this to the vNext milestone Feb 20, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 20, 2017

Coverage Status

Coverage increased (+0.6%) to 66.497% when pulling 3d3890f on kentcb:default-view-locator2 into 01c6af2 on reactiveui:develop.

@kentcb kentcb merged commit 427885e into reactiveui:develop Feb 20, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.6%) to 66.497%
Details

@kentcb kentcb deleted the kentcb:default-view-locator2 branch Feb 20, 2017

@kentcb kentcb referenced this pull request Feb 22, 2017

Merged

fix: prefer runtime type when resolving views #1288

3 of 3 tasks complete
@bradphelan

This comment has been minimized.

Copy link
Contributor

bradphelan commented Apr 24, 2017

It broke my code. My view models were never named *ViewModel as I didn't need the ViewModel noise at the end of every function. However

    public DefaultViewLocator(Func<string, string> viewModelToViewFunc = null)
    {
        ViewModelToViewFunc = viewModelToViewFunc ?? (vm => vm.Replace("ViewModel", "View"));
    }

maps Foo to Foo and it thinks it has now found a view. Not good

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