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

DocumentClosed event issue #195

Closed
Skaptor opened this issue Sep 2, 2020 · 6 comments · Fixed by #212
Closed

DocumentClosed event issue #195

Skaptor opened this issue Sep 2, 2020 · 6 comments · Fixed by #212

Comments

@Skaptor
Copy link
Contributor

Skaptor commented Sep 2, 2020

I found that from v4.30 and up, the content property of the closing document is null, so mi PRISM app cannot remove the region since the content is null.

private void DockingManager_DocumentClosed(object sender, DocumentClosedEventArgs e) { LayoutDocument layoutDocument = e.Document; //e.Document.content is null, breaking my region behavior using prism }

@Dirkster99
Copy link
Owner

Dirkster99 commented Sep 4, 2020

Hi,

I don't have a sample client to verify your issue :-( and so I am left to guessing at this point - but since there is:

  • only one code point that fires the DocumentClosed event in the DockingManager.ExecuteCloseCommand() method and
  • and only one change in the version window 4.30 and up

I am wondering if this change could be the cause of this issue.

Would you be able to test this with the current version 4.4 (with reference to source code project) and tell me if your issue can be fixed if we move the call to RemoveLogicalChild(uIElement); after the Event being raised (or if you see a better fix let me know also)?

internal void ExecuteCloseCommand(LayoutDocument document)
{
  if (DocumentClosing != null)
  {
    var argsClosing = new DocumentClosingEventArgs(document);
    DocumentClosing(this, argsClosing);
    if (argsClosing.Cancel) return;
  }

  if (!document.CloseDocument()) return;
  
  RemoveViewFromLogicalChild(document);
  
  DocumentClosed?.Invoke(this, new DocumentClosedEventArgs(document));
  
  if (document.Content is UIElement uIElement)
      RemoveLogicalChild(uIElement);
}

@Skaptor
Copy link
Contributor Author

Skaptor commented Sep 4, 2020

I think the issue is at the CloseInternal() function in LayoutContent.cs as the content is explicitly set to null.

internal void CloseInternal()
{
  var root = Root;
  var parentAsContainer = Parent;

  if (PreviousContainer == null)
  {
    var parentAsGroup = Parent as ILayoutGroup;
    PreviousContainer = parentAsContainer;
    PreviousContainerIndex = parentAsGroup.IndexOfChild(this);

    if (parentAsGroup is ILayoutPaneSerializable layoutPaneSerializable)
    {
      PreviousContainerId = layoutPaneSerializable.Id;
      // This parentAsGroup will be removed in the GarbageCollection below
      if (parentAsGroup.Children.Count() == 1 && parentAsGroup.Parent != null && Root.Manager != null)
      {
        Parent = Root.Manager.Layout;
        PreviousContainer = parentAsGroup.Parent;
        PreviousContainerIndex = -1;

        if (parentAsGroup.Parent is ILayoutPaneSerializable paneSerializable)
          PreviousContainerId = paneSerializable.Id;
        else
          PreviousContainerId = null;
      }
    }
  }

  parentAsContainer.RemoveChild(this);
  root?.CollectGarbage(); 
  this.Content = null;  //<--------------------------- issue is here
  OnClosed();
}

I wonder if the ExecuteCloseCommand function can be changed in order to invoke the DocumentClosed event using a new "DocumentClosedEventArgs" variable, since I need to see which content is being "closed" in order for the PRISM framework to remove it from the region?

Thank you

**EDIT: had mixed up the variable type suggestion (DocumentClosedEventArgs)

@Dirkster99
Copy link
Owner

Dirkster99 commented Sep 6, 2020

Hi @LyonJack
it turns out that we need to preserve the content in the DocumentClosed event since PRISM needs to see the content that was closed. Do you see a good fix for adjusting your leak fix #171 such that the content can still be part of the event?

@Dirkster99
Copy link
Owner

@LyonJack
I have received no answer from @LyonJack on this problem so I am wondering if @Skaptor can suggest a solution that will set the content to null after having messaged the Document Closed Event? If so, please feel free to descripe the suggested change in detail or prepare a PR please, thank you :-)

@Skaptor
Copy link
Contributor Author

Skaptor commented Sep 24, 2020

@Dirkster99 Sure, I will see what I can do without breaking something, my initial guess is to invoke the event before getting rid of the content object. I will keep you posted! Thank you. :D

@Skaptor
Copy link
Contributor Author

Skaptor commented Nov 26, 2020

@Dirkster99 Hello Dirkster, I will post a PR with the modified code, I ran the unit test project, but is there a way to see if I didn't break anything else??

image

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 a pull request may close this issue.

2 participants