The old implementation had a few problems:
- For a target area near but not touching the bottom of the parent area,
and a long menu with an initially-visible item halfway through the
list, the initially-visible item would sometimes be obscured by the
scroller area. The new implementation adjusts the size of the menu to
ensure that there's enough additional room for the scroller areas.
- For a target area at the very top or very bottom of the target area,
the menu would be positioned inconsistently; sometimes it would
overlap with the target area, but other times it would be positioned
with a gap separating the menu and target area. In the new
implementation, if there's not enough room for scrollers to be
positioned above/below the target area, the menu will always be
positioned so that it touches but does not overlap the target area.
- The initially-selected item would normally be
positioned as close as possible to the target area, but this wasn't
always applied consistently for long menus, and the menu would
sometimes only be scrolled enough to make the item visible at the end
of the menu furthest from the target area. In the new implementation,
the initially-visible item will always be positioned on top of the
target area, or adjacent to it, depending on the available space.
This makes it a bit easier to see exactly which PopupMenu state is used
during the calculation, and enforces that no menu state is modified
during the call.
The issue became manifest in c51b331318.
In our implementation the toggle action takes precedence over the press
action, making the latter unreachable when the Item is in a checkable
state. Calling isTicked (true) turns the Item into a checkable object.
The onToggle implementation however didn't interact with the isTicked
state, and it didn't fire the press action either. This made the item
non-interactable with screen readers once it got into a ticked state.
This reimplements similar functionality to that removed in
d39789b021.
Unlike the previous implementation, this version will still wait for the
mouse to move over the menu before the countdown timer is allowed to
start. This should avoid the situation where menu items are accidentally
triggered on mouse-up, after a menu opens under the pointer in response
to a mouse-down.
We now require a mouse movement before highlighting menu items. This is
intended to ensure that the user has intentionally highlighted the menu
item that they wish to trigger.
This makes behaviour more consistent between the parented and
non-parented display modes.
Before this change, parented popup menus wouldn't respond to mouse
clicks until the mouse was released once, which meant that clicking and
dragging the mouse over a menu wouldn't trigger a menu item on mouse-up.
Previously, MouseSourceState::checkButtonState would trigger a menu item
if the MouseSourceState had observed the mouse button transition from
pressed to released while over an item, after more than 250ms had
elapsed since creating the menu window. In situations where the main
thread was very busy, this timeout could sometimes be reached inside the
same mouse click/release gesture. If the menu was created inside a
mouse-down, then simply tapping the mouse could sometimes trigger an
item from the menu as soon as the menu window appeared.
To help avoid accidentally triggering menu items, the menu window now
prevents any item from being triggered by the mouse until either the
mouse has been released once, or the mouse has moved. Put another way,
if the mouse is initially pressed when the menu is shown, it cannot
trigger a menu item unless the mouse is moved before it is released.
This allows the LookAndFeel of submenus to query the target component
used for the top-level menu. getTargetComponent() isn't suitable for
this because the target component is set to null for submenus, and this
behaviour can't be changed without potentially breaking code that relies
on the current behaviour.
In cases where no JUCE component had keyboard focus, clicking a menu
item would cause the menu to be dismissed instead of triggering the
item's action.
- Created a new detail namespace
- Moved shared module implementation details into the detail namespace
- Split dependencies so source files only rely on details in the detail namespace
- Removed all code from the juce_gui_basics.cpp file
Without this VoiceOver will iterate over menu items left to right first,
and iteration order will be affected by whether the PopupMenu gets
broken up into multiple columns due to not enough screen space.
Previously, for the following snippet, the menu's LnF was incorrectly
being forced to the default LnF. The correct behaviour is to display the
menu using LnF v4. The menu doesn't have an explicit LnF set, so it
should use the LnF of its parent component.
LookAndFeel::setDefaultLookAndFeel (&lookAndFeel_V1);
setLookAndFeel (&lookAndFeel_V4);
PopupMenu().showMenuAsync (PopupMenu::Options{}.withParentComponent (this));
Prior to this commit it was possible to get the menu bar deactivated
by moving the mouse to an adjacent menu item and then back again. If
the movement was quick enough the corresponding PopupMenu would be
dismissed and created again before the dismissal's async command
handler would run. The command handler would see that the dismissed
menu's index and the currently activated index are equal and
deactivate the menu bar.
Previously it was possible to inadvertently activate a menu item by
clicking on a submenu item that was drawn on top of the parent menu.
The root cause was that hide() initiates an asynchronous mechanism
through exitModalState() that eventually destroys the MenuWindow, but
the MouseSourceState timer callbacks and event handlers sometimes still
had a chance to do a state update. Since the submenus have just been
destroyed the update could mistakenly conclude to activate one of the
items of the now lone parent.
The 'wrapper' accessibility handler is now ignored if a menu item has a
custom component, and has no submenu, and cannot be triggered
automatically. This avoids the case where a custom menu item may end up
with a wrapper accessibility handler that has no useful actions.
This patch also adds a 'label' argument to the addCustomItem functions,
which allows text for the screen reader to be supplied in the case where
a custom component is in use, but the menu item has accessibility
actions.
