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

subModelWin issue passing a parameter #555

Closed
minewarriorsSchool opened this issue Feb 20, 2023 · 11 comments
Closed

subModelWin issue passing a parameter #555

minewarriorsSchool opened this issue Feb 20, 2023 · 11 comments

Comments

@minewarriorsSchool
Copy link

minewarriorsSchool commented Feb 20, 2023

So I currently have a small issue with my current program after making a change that cleaned up my model by removing UI objects from it.

However, this change resulting in changing my mapOutMsg for my subModelWindows from (just one example)

let mapOutMsg = function | ReplaceResourceWindowOutMsg.Close -> CloseReplaceResourceWindow'(false,) | ReplaceResourceWindowOutMsg.CloseDataChanged -> CloseReplaceResourceWindow'(true,)

to

let mapOutMsg = function | ReplaceResourceWindowOutMsg.Close gridControl-> CloseReplaceResourceWindow'(false, gridControl) | ReplaceResourceWindowOutMsg.CloseDataChanged gridControl -> CloseReplaceResourceWindow'(true, gridControl)

When changing this, my bindings in my bindings functions turned all red with the following example:
image

I am not sure on how I would go about this and fix this.
for your idea why I need that gridControl passed to it is becuse of the following example:
image

Hopefully you can help me out figuring out how I can fix this.

@minewarriorsSchool minewarriorsSchool changed the title subModelWin, pass argument to mapOutMsg subModelseq Feb 20, 2023
@minewarriorsSchool minewarriorsSchool changed the title subModelseq subModelWin issue passing a parameter Feb 20, 2023
@TysonMN
Copy link
Member

TysonMN commented Feb 20, 2023

If you provide a link to a GitHub repo containing a minimal working example, then I can show you how I would handle this.

@minewarriorsSchool
Copy link
Author

let me see what I can do @TysonMN . Probably will cost me a day, but I will mention you when I finished it.

@minewarriorsSchool
Copy link
Author

@TysonMN I have send you a invite for the project and reproduced it to the most simplified version I think
https://github.com/minewarriorsSchool/ElmishWpfGithubProject/invitations

@minewarriorsSchool
Copy link
Author

Hi @TysonMN ,

You are probably really busy, but I wondered if you had a chance already to have a peek at the sample project?
Your time, effort and help is really much appreciated.

Kind regards,

Jelle

@TysonMN
Copy link
Member

TysonMN commented Feb 28, 2023

Hello Jelle.

Yes, still busy. I haven't had a chance to look yet. I will, but I am not sure when right now. Sorry for the long delay.

@LyndonGingerich
Copy link
Contributor

Okay, here's an issue. The type signature of TestWindow.bindings is

unit -> Binding<NewTestWindow,InOut<NewTestWindowMsg,(obj -> Window2OutMsg)>> list

whereas I suspect that you want

unit -> Binding<NewTestWindow,InOut<NewTestWindowMsg,Window2OutMsg>> list

(It's obj on my machine because DevExpress.Wpf was not found on NuGet. Where I see obj, you will see GridControl instead.)

So the out-message type is a function when it should be just a message value.

All out-message bindings in this list are made using TestWindow.confirmStateToMsg and, sure enough, its type signature is ConfirmState -> 'a -> NewTestWindow -> InOut<'a,(obj -> Window2OutMsg)>; it's returning a function into InOut.Out.

Looking inside TestWindow.confirmStateToMsg, we see that the out-message returned is Window2OutMsg.Close. Sure enough, it's a function, not a simple value.

image

Pass a GridControl into Window2OutMsg.Close there and you'll be golden.

@LyndonGingerich
Copy link
Contributor

Here's what tipped me off:

image

The "Known types of arguments" section, when examined closely and perhaps compared with the first overload or two, will almost always reveal the issue.

@minewarriorsSchool
Copy link
Author

minewarriorsSchool commented Mar 1, 2023

@LyndonGingerich oooeeeeh facepalm.........
That makes a lot of sense... #tunnelvision 🤣

Let me see if this indeed is the case on my main project (pretty large codebase to fix it for) and if I am able to solve it.
I will keep you up to date! ♥️

@minewarriorsSchool
Copy link
Author

@TysonMN @LyndonGingerich The issue indeed was that I forgot to pass a gridControl to the InOut of confirmStateMessage.

So how I solved it was to

  • add a "grid" parameter to the comfirmstateToMessage method
  • give this grid parameter to the close and closedatachanged state OutMessages
  • make the submit binding a Binding.cmdParam that converts the given param to a gridcontrol
  • give the correct message and grid to the confirmStateToMessage method.

Thanks a lot for helping me out. After staring at this for so long I could not see what was wrong.
In the end the solution was such an obvious one hahaha.
#ProgrammerTunnelVisionIssues 🤣

@LyndonGingerich
Copy link
Contributor

Yup, I've been there before. Glad it's figured out!

@TysonMN
Copy link
Member

TysonMN commented Mar 3, 2023

@LyndonGingerich, thanks for the help!

@minewarriorsSchool, glad you solved your problem.

In the future, when you have code that doesn't type check like that, then I suggest you extract pieces to local values. Initially have the type inferred (i.e. use a let binding without a type annotation), and then consider if that is the type you intended. Then consider adding a type annotation (so the inferred type doesn't change as you continue this process).

As an example, consider the code in the screenshot in the above comment by @LyndonGingerich. That is showing a type check error in a list of bindings, so applying the above process involves extacting each binding to its own value.

More specifically, consider the SimpleCounter bindings.

let bindings () : Binding<Model, Msg> list = [
  "CounterValue" |> Binding.oneWay (fun m -> m.Count)
  "Increment" |> Binding.cmd Increment
  "Decrement" |> Binding.cmd Decrement
  "StepSize" |> Binding.twoWay(
    (fun m -> float m.StepSize),
    int >> SetStepSize)
  "Reset" |> Binding.cmdIf(Reset, canReset)
]

If this didn't type check, then I am suggesting that could extract one binding at a time to a value...like this:

let counterValue = "CounterValue" |> Binding.oneWay (fun m -> m.Count)

let bindings () : Binding<Model, Msg> list = [
  counterValue
  "Increment" |> Binding.cmd Increment
  "Decrement" |> Binding.cmd Decrement
  "StepSize" |> Binding.twoWay(
    (fun m -> float m.StepSize),
    int >> SetStepSize)
  "Reset" |> Binding.cmdIf(Reset, canReset)
]

and then (possibly) give it a type annotation like this

let counterValue : Binding<Model, Msg> = "CounterValue" |> Binding.oneWay (fun m -> m.Count)

let bindings () : Binding<Model, Msg> list = [
  counterValue
  "Increment" |> Binding.cmd Increment
  "Decrement" |> Binding.cmd Decrement
  "StepSize" |> Binding.twoWay(
    (fun m -> float m.StepSize),
    int >> SetStepSize)
  "Reset" |> Binding.cmdIf(Reset, canReset)
]

Hopefully you typically write code by making small(ish) changes. In that case, you will start with an existing list of bindings that does type check and then adding a new binding causes things to not type check. Then you should extract that binding first.

Also, it is helpful to remember that F# infers the type of a list by the first element in the list. So

let x = [ "foo", 0 ]

has inferred type string list while

let x = [ 0, "foo" ]

has inferred type int list. For this reason, it is typically clearer to always add new code NOT at the beginning of a list. For example, if you started with the int list

let x = [ 0, 1, 2, 3, 4, 5 ]

and then added "foo" to the beginning to get

let x = [ "foo", 0, 1, 2, 3, 4, 5 ]

then the inferred type is now string list so "foo" type checks while all the other values do not pass type checking, which can be confusing because your attention is drawn to those ints, but the "real" problem is the first element in the list, which passed type checking.

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

No branches or pull requests

3 participants