Friday, June 10, 2016

Fixing the Edit Menu Fix

A year and a half ago, I wrote an incomplete description of my solution for separate edit menus. While using PPQT2 in Windows 7 to test the logging/dictionary bug, I noticed a problem with this system and fixed it.

To review, under Qt there is a single menu bar that is owned by the main window. (The main window is a parentless widget based on the QMainWindow class.) In the menu bar there can be just one Edit menu.

In the Qt system, any menu is populated by QAction objects. In the Edit menu, if there is, say, a "Cut" item, it is installed in the menu by adding a QAction('Cut'). Any QAction has four properties:

  • A name string, like 'Cut'
  • An optional icon
  • An optional accelerator keystroke, like control-x
  • A "slot", which is an executable

The slot is the important item. It is a function that receives the "triggered" signal when the user selects that menu item or keys that accelerator.

This is a fine system, provided that there is no ambiguity about which slot to call. In an app with a simple UI, the one main window perhaps, the slot for File:Save or Edit:Cut is a method of the QMainWindow object, or of some sub-widget that it owns, and that never changes.

In PPQT2, there are multiple different widgets that can implement Edit actions. In the simplest case, the user has a single book open. But there is an Edit panel with the book text, a Notes panel with a separate simple document to edit, and a Words panel which supports copying selected words from a table. If the focus is in the Edit panel, we want the Edit:Cut action to be implemented by the cut() method of the QPlainTextEdit object that is part of that panel. But if the focus is in the Notes panel, we want Edit:Cut to be implemented by the cut() method that is part of the QPlainTextEdit object that is part of that panel. And if the focus is in the Words panel, we don't want to implement Edit:Cut at all, but we do want to support Edit:Copy via a method of the QTableView that implements the vocabulary table.

Worse: if the user has two or three books open, each book has its own Edit, Notes and Words panels. When the user hits control-x, it is important that the cut be implemented by the Edit or Notes panel for the book that is in front -- not by some panel containing the text of a different book that isn't visible.

The only solution is to continually change out the actions in the one-and-only Edit menu whenever the focus changes. When the Notes panel gets focus, it loads the Edit menu with the set of actions it supports, including slots that are methods of that unique Notes widget (distinguished by the "self" parameter). The code to do this, before this week, was as follows:

def set_up_edit_menu( action_list ) :
    global _EDIT_MENU, _LAST_MENU

    _EDIT_MENU.setEnabled(True)
    if id( action_list ) != _LAST_MENU :
        _LAST_MENU = id( action_list )
        _EDIT_MENU.clear()
        for (title, slot, key) in action_list :
            if title is None :
                _EDIT_MENU.addSeparator()
            else :
                _EDIT_MENU.addAction(title, slot, key)
def hide_edit_menu():
    global _EDIT_MENU
    _EDIT_MENU.setEnabled(False)

Any widget that wanted to support the edit menu had to do three things. It had to set up a list of Edit menu actions it supported, including the title, the slot, and a key code for each. It had to catch the focus-in event and call set_up_edit_menu() with that list. It had to catch the focus-out event and call hide_edit_menu(). As a result the Edit menu would be disabled (grayed-out) whenever focus was in a widget that didn't support it. When focus entered a widget that did support the Edit menu, it would be enabled and populated with that widget's actions, including references to that widget's slots.

I observed that it was quite frequent for a widget to get a focus-out followed by a focus-in, with no intervening visit to another widget. That's why the work of changing the menu is only done if the id() of the list is different. The Python id() of an object is basically its memory address. In my first description of this code, I was requiring the caller to pass a unique key, but I soon figured out that was not needed; id() did the job.

This all worked fine under Mac OS. Unfortunately I'd never really checked that it worked under Windows or Linux. This week I noticed that it did not. (imagine a blushing embarrassed emoji here)

In Windows and under Ubuntu Unity at least, the Edit menu was disabled, grayed-out, all the time! Why?!? The contents of the menu were changing properly, for example when focus was in the Edit panel, the menu contained the to-Upper and to-Lower actions that only the Edit panel supports. They went away when the focus moved to the Notes panel. So set_up_edit_menu() was being called. Which meant _Edit_MENU.setEnabled(True) was being executed.

It took a good half-hour of swearing and inserting print() statements before I realized that hide_edit_menu() was being called when I clicked on the word Edit in the menu bar. (imagine an open-mouth amazed emoji)

It seems that in Windows (and in Ubuntu, I later found out), a click anywhere in the menu bar causes a change of focus! You click in the Edit panel; the focus-in event calls set_up_edit_menu(). You click on the word Edit in the menu bar and the Edit panel gets a focus-out event and immediately calls hide_edit_menu(). It's a beautiful catch-22: the menu is enabled all the time until you try to use the menu, then it is disabled.

It took some pondering to figure out how to fix this. My solution was to ask, in hide_edit_menu(), if the current mouse location was within the global menu bar. If so, do not disable the menu. Here's the new code:

def hide_edit_menu():
    global _EDIT_MENU, _MENU_BAR
    
    if not C.PLATFORM_IS_MAC :
        relative_cursor_pos = _MENU_BAR.mapFromGlobal( QCursor.pos() )
        if _MENU_BAR.geometry().contains( relative_cursor_pos ) :
            return
    # either this is Mac OS, or the cursor is not over the menu bar
    _EDIT_MENU.setEnabled(False)

That made the Edit menu behave the way I wanted in all three systems.

No comments: