TableHeaderComponent::addMenuItems is broken


#1

…and nobody has noticed for 12 years.

Current code:

void TableHeaderComponent::addMenuItems (PopupMenu& menu, const int /*columnIdClicked*/)
{
    for (auto* ci : columns)
        if ((ci->propertyFlags & appearsOnColumnMenu) != 0)
            menu.addItem (ci->id, ci->name,
                          (ci->propertyFlags & (sortedForwards | sortedBackwards)) == 0,
                          isColumnVisible (ci->id));
}

Fixed code:

void TableHeaderComponent::addMenuItems (PopupMenu& menu, const int /*columnIdClicked*/)
{
    for (auto* ci : columns)
        if ((ci->propertyFlags & appearsOnColumnMenu) != 0)
            menu.addItem (ci->id, ci->name,
                          true,
                          isColumnVisible (ci->id));
}

This menu is for hiding / showing columns, not sure why it’s only enabled if the column is not sorted forward or backwards.


#2

Request to juce team: When you fix bugs can you post to thread and say they are fixed. Last few bugs I’ve reported got no response so I assumed they had been ignored and didn’t notice when they were fixed.


#3

I think the intention here is that you shouldn’t be able to hide the column which is currently being used to sort the list (either forwards or backwards), so it’s expected behaviour that this column is disabled in the popup menu.


#4

That is odd expected behavior, it’s not obvious to the user why they can’t hide the column. Similar controls on Windows and macOS don’t have that restriction.

This is my setup bug, but the TableHeaderComponent allows multiple columns to have their sort flag set. They won’t get cleared until the sort changes for the first time.