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

file manager pop-up function window never disappear #1036

Closed
kkbruce opened this issue May 16, 2023 · 8 comments
Closed

file manager pop-up function window never disappear #1036

kkbruce opened this issue May 16, 2023 · 8 comments
Assignees
Labels

Comments

@kkbruce
Copy link

kkbruce commented May 16, 2023

mm version 2.9.6

If I right-click on any file in the file manager, the pop-up function window will never disappear, even if I have clicked on any of the functions inside.

image

@RickStrahl RickStrahl self-assigned this May 16, 2023
@RickStrahl RickStrahl added the bug label May 16, 2023
@RickStrahl
Copy link
Owner

Ugh - yes I see it too now...

Not sure what's causing this though - the folder browser code hasn't been touched in quite some time so not sure what changed in the context menu activation. I'll have to take a closer look.

@RickStrahl
Copy link
Owner

So after a lot of back and forth and checking the Git change log I tracked this down to a change in the app.manifest and the DPI settings that were recently changed.

Specifically this throws the menu into the wrong place (0 offset):

  <application xmlns="urn:schemas-microsoft-com:asm.v3">
    <windowsSettings xmlns:ws2="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
        <ws2:longPathAware>true</ws2:longPathAware>

        <dpiAware xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">true</dpiAware>
        <ws2:dpiAwareness>PerMonitorV2</ws2:dpiAwareness>
    </windowsSettings>
  </application>

If I comment out the the dpiAware settings then the menu works correctly:

  <application xmlns="urn:schemas-microsoft-com:asm.v3">
    <windowsSettings xmlns:ws2="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
        <ws2:longPathAware>true</ws2:longPathAware>

        <!-- <dpiAware xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">true</dpiAware>
        <ws2:dpiAwareness>PerMonitorV2</ws2:dpiAwareness> -->
    </windowsSettings>
  </application>

This is very strange! Especially since there are many other context menus that essentially work the same as this one - the tab, editor and preview context menus are working just fine with the dpi settings enabled. It's only the FolderBrowser context menu that's affected. I suspect it's due to a control reference that maybe isn't valid or pointing at an element that doesn't have a context menu.

For now I've recompiled with the old manifest setting until I can figure out why that's failing.

Update should be in 2.9.7 shortly.

@RickStrahl
Copy link
Owner

RickStrahl commented May 16, 2023

So I spent a bit more time looking into this problem.

It seems this is caused by the fact that MM in the folder browser manually intercepts all mouse operations (due to the lousy mouse event handling inside of the native Treeview control). All of this resulted in multiple events firing and competing for the context menu event handling.

I ended up refactoring a bunch of the click handling code. It appears that the main difference is a timing and focus issue where if focus is not on the form and item exactly, the context menu jumps out. The ultimate fixes were two things:

  • Removing explicit click handling of context menu items and let OnContextMenuOpening work.
  • Explicitly delay opening the menu using a Dispatcher() call after focus is set.

In the end I was able to consolidate a bunch of right click handling code into something much simpler:

private void TreeViewItem_PreviewMouseDownClick(object sender, MouseButtonEventArgs e)
{
    startPoint = e.GetPosition(null);

    if (e.RightButton == MouseButtonState.Pressed)
    {
        var item = ElementHelper.FindVisualTreeParent<TreeViewItem>(e.OriginalSource as FrameworkElement);
        if (item != null)
        {
            TreeViewSelection(item);
            e.Handled = true;
        }

        TreeFolderBrowser.ContextMenu = FolderBrowserContextMenu.ContextMenu;
        FolderBrowserContextMenu.ShowContextMenu();
    }
}

And then for manually invoked menu:

public void Show()
{
    ContextMenuOpening?.Invoke(this, ContextMenu);
    if (ContextMenu != null && ContextMenu.Items.Count > 0)
    {
        Model.ActiveEditor?.SetMarkdownMonsterWindowFocus();

        FolderBrowser.Dispatcher.Invoke(() =>
        {
            ContextMenu.IsOpen = true;

            var item = ContextMenu.Items[0] as MenuItem;
            item?.Focus();
        }, DispatcherPriority.Background);
    }
}

The key here is the dispatcher which ensures that the menu isn't shown until the editor/window has focus (even though we're not in the editor).

As a bonus the context menu now behaves much cleaner - it snaps into place without the previous flicker and occasional missed right click.

Sweet!

If all of this seems too custom: MM does a whole bunch of custom key and click handling and the default TreeView click and keyboard events don't offer enough granualarity to handle selection management. So there's a lot going on and it's often difficult to see all the interactions - this apparently was one of them with the thrown in curveball of the different behavior for the DPI setting.

The updated version now works with multi-monitor DPI enabled.

@kkbruce
Copy link
Author

kkbruce commented May 17, 2023

I'm looking forward to the arrival of a new version.

@RickStrahl
Copy link
Owner

RickStrahl commented May 17, 2023

2.9.7.2 now available for download

@kkbruce
Copy link
Author

kkbruce commented May 17, 2023

@RickStrahl Is the download 2.9.7.2 version the same mmReleases 2.9.7 version?

Because I am used to downloading and installing from the mmRelease by Winget function. I maintain MM for Winget :-)

@RickStrahl
Copy link
Owner

RickStrahl commented May 17, 2023

It's a different version. Wait until 2.9.8. 2.9.7 has the basic fix via the manifest. 2.9.7.2 has the additional tweaks for the folder browser. I'll let these run for a few days before updating to 2.9.8.

You're the WinGet maintainer? I guess my frequent releases keep you busy, huh?

@kkbruce
Copy link
Author

kkbruce commented May 18, 2023

  1. Understand. Then I will wait for 2.9.8 to test again. I will close this issue first.
  2. One small thing.

@kkbruce kkbruce closed this as completed May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants