Kuuuube / kanjigrid

Kanji Grid for Anki 23.10+ with ui improvements and bug fixes.
https://ankiweb.net/shared/info/1610304449
GNU Affero General Public License v3.0
13 stars 3 forks source link

Add context menu #11

Closed iamllama closed 3 weeks ago

iamllama commented 3 weeks ago

Closes #6 (got waylaid for a few weeks 😅)

~~EDIT: Also included a fix for #10. Apparently closeEvent isn't called when the dialog is rejected, i.e. when Close is pressed. There's also a small amount of memory stil being leaked, which I presume is the dialog itself. Calling mw.garbage_collect_on_dialog_finish seems to clear it up~~

Kuuuube commented 3 weeks ago

It would be nice to have the cleanup and memory leak stuff split out into a separate pr.

Kuuuube commented 3 weeks ago

Thank you for your work on this!

iamllama commented 3 weeks ago

Sorry just realised while working on another feature that the merge conflict was handled incorrectly

It's currently

        def on_window_close(_):
            gui_hooks.webview_will_show_context_menu.remove(self.add_webview_context_menu_items)
        self.win.closeEvent = on_window_close
        qconnect(self.win.finished, lambda _: self.wv.cleanup())
        mw.garbage_collect_on_dialog_finish(self.win)

when it should be

        def on_window_close(_):
            self.wv.cleanup()
            gui_hooks.webview_will_show_context_menu.remove(self.add_webview_context_menu_items)
        qconnect(self.win.finished, on_window_close)
        mw.garbage_collect_on_dialog_finish(self.win)

self.win.closeEvent was left in and the problem mentioned in #12 still applies, whereby the hook isn't cleaned up :/

Kuuuube commented 3 weeks ago

Thanks, pushed the fix.