TimLariviere / Fabulous-new

Fabulous v2 - Work in progress
https://timothelariviere.com/Fabulous-new/
Other
41 stars 3 forks source link

WIP started reworking MapMsg, should work e2e but haven't figured out a couple of details yet #22

Closed twop closed 2 years ago

twop commented 2 years ago

fixes https://github.com/TimLariviere/Fabulous-new/issues/11

from our conversation on Discord

@TimLariviere I'm trying to make a nicer MapMsg approach and I noticed an interesting thing [4:39 PM] we calculate the diff recursively and all at once [4:40 PM] e.g. we calculate a potentially large data sctructure that represents the diff for the entire tree [4:42 PM] I haven't reconciled all the pros and cons of it but there are a couple of thoughts that came to my mind:

  1. Allocating a big diff can represent a perf issue. Not sure if it is relevant or going to be just fine in practice.
  2. ViewNode doesn't have a reference to a widget it represents currently [4:43 PM] so 2. is relevant for me in the context of MapMsg work: there is no easy way to access scalar attributes for a view node at the moment of dispatch [4:45 PM] I think I will add a temprorary hack of adding current scalar attribute to a view node, but I'm curious if there is a more elegant solution [4:47 PM] like so abstract ApplyScalarDiff : struct (ScalarChange[] * ScalarAttribute[]) -> unit

Simon Korzunov — Today at 11:28 PM Upd: I was able not to do that [11:29 PM] instead I just uses the attribute update function to set IViewNode.MapMsg [11:30 PM] so with that and adding Ancestors to ViewTreeContext I was able to achieve the goal [11:31 PM] now when we dispatch a message it goes through the parent chain and maps the message along the way [11:31 PM] What I haven't figured out yet is how to do it the way you wanted with typesafe children (like ITabbedPage) [11:34 PM] What I would love to do but haven't figured out yet is this:

[] static member inline AddScalar_(this: ^T :> IWidgetBuilder, attr: ScalarAttribute) = let newAttribs = ... let result = (^T: (new : ScalarAttribute [] WidgetAttribute [] WidgetCollectionAttribute [] -> ^T) (...)

    result

but (^T: (new : ScalarAttribute [] WidgetAttribute [] WidgetCollectionAttribute [] -> ^T) (...) call would change the type of the msg to a new one [11:35 PM] technically it should be possible to achieve because the actual message type is phantom, but my typefoo is not strong enough [11:35 PM] will have a PR in a sec [11:37 PM] https://github.com/TimLariviere/Fabulous-new/tree/simon/rework-map-msg

[11:38 PM] @TimLariviere I haven't checked if this works, could you verify on Contacts? It should all work e2e If it does I suggest to merge it, if you onboard with changes it would be awesome if you do so. Let me create a PR for that

edgarfgp commented 2 years ago

This PR introduces some changes regarding the foundation to how we define Events . I think we should merge this first . I can rebase #19 and deal with the new changes there :)

TimLariviere commented 2 years ago

@twop I'll give it a try with FabulousContacts and let you know