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

Extract a ViewModelHelper that will be useful with static view models #523

Merged
merged 1 commit into from
Oct 29, 2022

Conversation

marner2
Copy link
Collaborator

@marner2 marner2 commented Oct 3, 2022

Should rebase easily after #535

This should be a behavior-preserving transform that extracts some behavior from DynamicViewModel into a ViewModelHelper which can then be shared with a future ViewModelBase implemented in #522

@TysonMN
Copy link
Member

TysonMN commented Oct 3, 2022

The diff has huge blocks of + and -, which is very difficult to review on my phone. Is it possible to extract smaller pieces in separate PRs?

@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from f6befc9 to 17ae441 Compare October 4, 2022 19:23
@marner2
Copy link
Collaborator Author

marner2 commented Oct 4, 2022

@TysonMN can you view the diffs one commit at a time? A lot of these changes don't make much sense on their own.

@marner2
Copy link
Collaborator Author

marner2 commented Oct 4, 2022

Also the majority of the diff issue happens when I split the class in two at the end, and I don't think I can help that because the diff view doesn't track code reordering (which must be done at that time and can't be reduced any more that I can see).

@TysonMN

This comment was marked as outdated.

@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from e6e355a to 5743879 Compare October 12, 2022 01:50
@marner2 marner2 marked this pull request as ready for review October 12, 2022 01:50
@marner2 marner2 marked this pull request as draft October 12, 2022 02:29
@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from 5743879 to 1e1cfea Compare October 12, 2022 03:51
@marner2 marner2 marked this pull request as ready for review October 12, 2022 03:52
src/Elmish.WPF/DynamicViewModel.fs Show resolved Hide resolved
src/Elmish.WPF/DynamicViewModel.fs Outdated Show resolved Hide resolved
src/Elmish.WPF/DynamicViewModel.fs Outdated Show resolved Hide resolved
@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from 1e1cfea to 77fdba1 Compare October 15, 2022 20:03
@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from 77fdba1 to b224d3e Compare October 16, 2022 01:11
@TysonMN TysonMN force-pushed the feature/extract_view_model_helper branch from b224d3e to bd7fd0b Compare October 16, 2022 03:23
@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from bd7fd0b to bab8b19 Compare October 16, 2022 05:35
@TysonMN TysonMN force-pushed the feature/extract_view_model_helper branch from bab8b19 to 73456a3 Compare October 16, 2022 19:33
@marner2

This comment was marked as outdated.

@TysonMN

This comment was marked as outdated.

@marner2

This comment was marked as outdated.

@marner2

This comment was marked as outdated.

@TysonMN

This comment was marked as outdated.

@marner2

This comment was marked as outdated.

@marner2 marner2 force-pushed the feature/extract_view_model_helper branch from 73456a3 to e20870b Compare October 19, 2022 22:07
@marner2

This comment was marked as outdated.

@TysonMN TysonMN force-pushed the feature/extract_view_model_helper branch from 52a8dab to ac93865 Compare October 29, 2022 12:02
@TysonMN TysonMN merged commit 6a622fb into elmish:master Oct 29, 2022
@marner2 marner2 deleted the feature/extract_view_model_helper branch October 30, 2022 20:40
@marner2 marner2 mentioned this pull request Nov 5, 2022
@@ -34,6 +34,75 @@ module internal IViewModel =
let currentModel (vm: #IViewModel<'model, 'msg>) = vm.CurrentModel
let updateModel (vm: #IViewModel<'model, 'msg>, m: 'model) = vm.UpdateModel(m)

type internal ViewModelHelper<'model, 'msg> =
Copy link
Member

Choose a reason for hiding this comment

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

Would a better name be ViewModelState?

src/Elmish.WPF/DynamicViewModel.fs Show resolved Hide resolved

let mutable helper =
ViewModelHelper.create
(fun () -> this)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this a function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was originally to avoid using references to this in the initialization section, but I'm not sure anymore.

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.

2 participants