[Bug]: ModalProvider loses parameter reference when closing popup

[drma-tech]

Blazorise Version

all

What Blazorise provider are you running on?

Bootstrap4

Link to minimal reproduction, or a simple code snippet

https://github.com/drma-tech/LostReferencePopup/tree/master/LostReferencePopup

Steps to reproduce

https://www.awesomescreenshot.com/video/24634047?key=6e7dea7043e561daaea2891e9c94e7fd

What is expected?

the value/reference should only be changed intentionally

What is actually happening?

When the popup is opened by the service (this does not happen with the embedded popup) the parameters that were loaded in this call are reset when the popup closes.

What browsers are you seeing the problem on?

Chrome

Any additional comments?

No response

[David-Moreira]

Indeed, currently the ModalProvider does not keep state very well. As when it needs to re-render modals it does not reuse the RenderFragment.

There's this related issue, where we want to make it so it's aware of the RenderMode that is set and manages state accordingly : #4612

[drma-tech]

@David-Moreira This has been open for exactly one year. Are you going to fix this now or leave it for later? because if it's not going to be fixed now, I need to find an alternative solution.

[drma-tech]

or if you have any workarounds, that works for me too.

[David-Moreira]

I'm not sure we should change the behaviour in middle of v1.4. Users might depend on how it behaves already, even if one could assume it should not behave like it does.

I'd do #4612 for v1.5 which should solve your problem in needing to keep state.

@stsrki thoughts?

---

Well in the meantime, for workaround... maybe you could keep the state in localStorage or something like that, and make sure that the ModalService parameters are loaded & saved in that centralized state? Would that be feasible?
Edit: Or some kind of state container the app is aware across the calls you need to make, where again, the parameters are loaded & saved from.

Other than that, the usage of regular Modals. But this can be a little more cumbersome depending on your needs.

I'll update you if I think of any more ideas.

[stsrki]

I'd do it in 1.5. It's not so far until release.

[drma-tech]

ok. i will wait

[David-Moreira]

@drma-tech
Refer to #5304

@stsrki can we revive nightly builds #5143 and have @drma-tech test the feature ahead of the release if he would be available to when we merge the feature in.

[stsrki]

@stsrki can we revive nightly builds #5143 and have @drma-tech test the feature ahead of the release if he would be available to when we merge the feature in.

We can try to make it on MyGet.

[David-Moreira]

@drma-tech this issue should be fixed with the introduction of #5304
The next release is scheduled for March.
I will go ahead and close this issue.

If you'd like to test the feature ahead of time and give us feedback, please use our dev packages for v1.5.0.
https://www.myget.org/gallery/blazorise

[drma-tech]

@David-Moreira

I still haven't been able to understand what I have to change in the code to maintain the popup state. I also didn't see anything in the documentation (is it a different domain?). Do you have any code examples I can use? You can also change the project I submitted here.

[David-Moreira]

At the moment we do not have documentation live for dev / pre-releases.
Only way is to run the documentation locally or to look at the corresponding pages in the source code:

Documentation/Blazorise.Docs/Pages/Docs/Services/ModalProvider/Examples/ModalProviderStatefulExample.razor

https://github.com/Megabit/Blazorise/blob/master/Documentation/Blazorise.Docs/Pages/Docs/Services/ModalProvider/ModalProviderPage.razor

[drma-tech]

@David-Moreira

Well, in addition to not solving the problem, the close of the first popup stops working.

From what I understand, this stateful maintains the popup reference for future uses, but this has no relation to the problem I reported.

The problem is not opening the popup, but closing the popup. When I close the second popup, the reference to the first is lost, and enabling stateful doesn't change this at all. Maybe what can solve this is the ElementId, but from what I understand it is only used in Show() and not in Hide().

Is there something I don't understand or is that it? I uploaded updates to the project so you can check.

[David-Moreira]

Hello @drma-tech
Thanks for taking the time to test the feature.
Maybe I miss understood your problem? I will take a look as soon as I have time.

[David-Moreira]

Hello @drma-tech
You are right, you do not have the same problem that we specifically fixed.

It seems like you are binding an object reference as the Parameter of the component that's rendered by the ModalProvider and then trying to change it by a new one.
Since the RenderFragment delegate already captured that one object reference you passed in, it will "rebind" it once it refreshes.
Would the following be okay for you? Leave the object reference be the same, but mutate the values inside it:
PageState.ActionNumber += (RefClass value) => { Ref!.Value = value.Value; StateHasChanged(); };

This should work and be a decent workaround for now?

---

As for the issue itself, I do believe you have a point, but we're just using regular blazor RenderTreeBuilder api internally,

I'm honestly not sure how could this specifically be handled.
PS: In theory, possibly it should be handled by providing a *Changed EventCallback to properly notify of the change, but that did not seem to work which is strange.
If any of you @drma-tech , @stsrki has any suggestion, I'll take it.
I'll retag this issue as Possible Bug & Investigate.

[drma-tech]

Why first popup is refreshing if I'm interacting only with the second one?

Note: change only a property, doesn't work for me, cause I need the whole object .

[drma-tech]

Is the component or the blazor that generates this refresh? If it's from the component, just avoid calling refresh, if I'm not interacting with the popup. If it's Blazor, you can keep these parameters in memory and pass them via callback or any other way.

[David-Moreira]

Is the component or the blazor that generates this refresh? If it's from the component, just avoid calling refresh, if I'm not interacting with the popup.

You would just be avoiding the issue if that's the case.
We pretty much just iterate over a collection of modals with the render fragment built from the blazor api render tree builder.

If it's Blazor, you can keep these parameters in memory and pass them via callback or any other way.

well that's how it's supposed to work... The reference is captured. Blazor recommends using the approach of EventCallbsck to notify parameter changes back to the consumer of the component, instead of mutating the value directly. (here the consumer would be the page that called the first pop-up)
There must be something to it. But I'm personally not sure. I would like to first make sure we understand what's going on fully and wouldn't like to create weird hacky workarounds if it's avoidable.

If you are that sure it's an easy fix, You are free to help us solve the issue by coming up with a PR if you'd like. We'd really appreciate it. :) Cheers.

[David-Moreira]

I was spending a little more time over this, I believe I found out the issue with both your approach and Blazorise.

In your approach if you want to make sure the reference is correctly updated, you will need to provide something like a
[Parameter] public EventCallback<RefClass?> RefChanged { get; set; } and propagate the value back to the consumer (The one that Shows the PopupBase)

This one needs to be told that the reference should now point somewhere else.
Something like this:

---

Don't bother reading next section, if you don't care about our technical details,
bottom line is, as long as you make sure the ref is correctly updated on your end, I believe we'll improve this binding as long we don't find technical issues since this does seem like an incorrect behavior

---

As for our side, I believe we should capture & re-evaluate the Action<ModalProviderParameterBuilder<TComponent>> parameters. (This is the fluent based helper we provide you with)

Currently we use it once, the time you click Show() to extract the parameters, and then it is pretty much a Dictionary that sends the boxed parameters into the Blazor internals. The problem is that this Dictionary was the only thing captured in the RenderFragment so it would always point to the same reference, even if someone updated the "pointer".

I made some successful attempts with something along these lines

I don't think the ModalProvider will be negatively affected in any way with this change, but I'll need to do some more testing later.

@stsrki any concerns? I'd patch this in v1.4 if I don't identify any issues.

[drma-tech]

I had tried this strategy of updating the original reference, but as it wasn't working, I imagined that you cut the binding and it was something internal to the popup, so I didn't know what to do to resolve it.

If you cut the binding, would it solve the problem? Like, what was passed to the popup will not be affected by anything external, unless it is intentional.

[David-Moreira]

I had tried this strategy of updating the original reference, but as it wasn't working, I imagined that you cut the binding and it was something internal to the popup, so I didn't know what to do to resolve it.

To resolve it as a temporary workaround I see two options:

• You keep the state somewhere else. Like your scoped service for instance.
• You create a CopyFrom method of some kind, where instead of changing the reference, you just copy the values from the other ref.

If you cut the binding, would it solve the problem? Like, what was passed to the popup will not be affected by anything external, unless it is intentional.

Can you rephrase this? I'm not sure what you mean.

We are thinking in re-evaluating the parambuilder you pass on to the ModalService which should be closer to how Blazor works in components. i.e
<MyComponent MyParameter=MyModelRef />
In the MyModelRef, blazor will continuously bind to the MyComponent the latest MyModelRef value.

We expect that ModalService.Show<MyComponent >(x=> x.Add("MyParameter", MyModelRef )) will do something similar by re-evaluating the x.Add("MyParameter", MyModelRef ) part when the RenderFragment delegate re-executes.

[drma-tech]

To resolve it as a temporary workaround I see two options:

You keep the state somewhere else. Like your scoped service for instance.
You create a CopyFrom method of some kind, where instead of changing the reference, you just copy the values from the other ref.

i already try to do that, but i couldnt figure out who is calling this change to be able to fix that
As you said, with this change in Show(), I would be able to force the original reference to be updated, it would be a good start.

Can you rephrase this? I'm not sure what you mean.

It's a crazy idea, I don't know if it will work. You said that changing Show() and forcing the original reference to update would correct this problem, so it means that the popup would have an active binding and would accept future changes. If, at the time of Show(), you take these parameters and duplicate this reference (with a shadow copy or something similar), the binding would be inactive and would not suffer external interference.

[drma-tech]

@David-Moreira

I updated to dev2. I made the changes you suggested, but it still doesn't work.

Have you tested these changes? I have already updated the project.

[David-Moreira]

Hello.
This is scheduled for the next patch for 1.4, since it was determined as a bug.

[David-Moreira]

Oh, but you are right in assuming the change would also be in master.

@stsrki have you merged in 1.4 patch to master yet, you usually do it on release day right?

[stsrki]

Yes, it's already merged into 1.4. But, I cam always unmerge it if you think it is worth it.

[David-Moreira]

Ah? I'm asking whether have you merged it to master yet. Since drma was trying to test it through our master dev release.
Or just give us an idea of the next 1.4 release patch.

[stsrki]

Ah, sorry, I misunderstand. I will merge it now and run the pipeline.

[stsrki]

It is now released as 1.5.0-dev-3.

[drma-tech]

ok, now its working. :)