Accessibility and Custom PopupMenu Items

Hi!

In surge we’ve made quite extensive use of custom components in menus. Here’s one of our modulation menus for instance. All works great.

Anyway we got a report from one of our beta testers using a screen reader that our menus have ‘blank’ items in them. When I read this with a screen reader (here using python atomacos to dump the screen hierarchy to stdout) I see this

MENU>>  'menu' (AXWindow actions=['Raise'])
MENU>> |-- 'Surge XT' (AXWindow actions=['Raise'])
MENU>> |--|-- '' (AXMenuItem actions=['Press'])
MENU>> |--|-- 'Pre-Filter Gain' (AXMenuItem actions=['Press'])
MENU>> |--|-- 'Edit Value: 0.00 dB' (AXMenuItem actions=['Press'])
MENU>> |--|-- '' (AXMenuItem actions=['Press'])
MENU>> |--|-- 'MODULATIONS' (AXMenuItem actions=[])
MENU>> |--|-- '' (AXMenuItem actions=['Press'])
MENU>> |--|-- 'ENV 3 by 4.63 dB' (AXMenuItem actions=['Press'])
MENU>> |--|-- 'Clear ENV 3' (AXMenuItem actions=['Press'])

So OK see that blank before Pre-Filter Gain, Modulations, Env 3 etc… Those are accessible wrappers for the Item which wraps the CustomComponent (the CustomCompponents themselves implement the Accessibility API and that works great)

So what’s happening is that little red x which maps to “Clear” is in a component with the rest of the things but the menu component hierarchy looks like

MenuComponent
    ItemComponent
        CustomComponetn and Children

and I’m getting that extra blank.

If i edit juce::PopupMenu::addItem(int, CustomCoponent *) to set i.text = "Foo" then these blanks all become Foo. This is clearly the extra parent component.

So:

Any ideas how to suppress this parent component from accessibility view? It makes our menus look pretty ugly to screen readers and I think others with custom component menus will face the same problem.

Thanks

I’ve been thinking about this today, but it turns out to be a bit tricky to get right. There doesn’t seem to be a way to bypass these ‘wrapper’ items at the moment, so there’s definitely room for improvement.

The main problem is that sometimes, we want the custom item to behave like a ‘normal’ menu item, so that it still allows highlighting, clicking, submenus and so on. e.g. this might be useful for a menu in which each item is an image, rather than some text. In this case, we want the ‘wrapper’ accessibility items to be present, so that the item can still be pressed, and open submenus. It would be a bug to have empty text in this case, though, so we should probably add an ‘accessibility text’ argument for the custom item.

I think we could also be a bit more selective about deciding when to create the ‘wrapper’ items. My idea is to check whether item.customComponent == nullptr || canBeTriggered (item) || hasActiveSubMenu (item) and to set the wrapper’s accessibility role to ‘ignored’ if the condition is not met. I think this will produce the results you’re looking for, as long as you set the custom component’s IDs to zero.

Do you think this approach would work for your use-case, or do you need to differentiate your custom components by their IDs for some reason? I’d be interested to hear your thoughts.

I agree this one is tricky. I spent a bit of time hacking in it and couldn’t quickly get a satisfactory answer

My gut is: bite the bullet and make it explicit. That is the custom component base class gets a method like virtual bool supressparentitemaccessibikity() return true (sorry I’m on my phone). I worry any heuristic would have edges we can’t predict.

Then I could override this but each user of the custom component would make a choice at accesibility time

See what I mean?

Oh and yeah I use -1 for all my ids and just bind lambdas and objects with callbacks in my case. But I think there’s lots of cases!

Oh and if adding to the base class you could also include a method for parent item accessible text, or alternately if the suppress method is false set the item text to custom component get title

Thanks for getting back to me. I think you’re right that making the behaviour slightly more explicit is probably the way to go, although I don’t really like the idea of adding a new function that is redundant in some cases (e.g. if the menu item has a submenu, we will always need to create the accessible wrapper).

I spent some more time looking at the problem and noticed that the CustomComponent class already has a triggeredAutomatically property that controls whether or not pressing the menu item triggers it and closes the menu. This seems to closely match the behaviour that we want for the accessibility handler wrapper - it should only be present if the item is automatically triggerable (or has a submenu).

I think we could probably use this same property to control whether or not the accessible wrapper is generated. This would mean passing false to the CustomComponent constructor for each of your custom menu items. Then, we’d still need a way of passing a title for the wrapper when necessary. Here, my preference is to add an argument to addCustomItem so that we can use the existing Item::text field, rather than expanding the CustomComponent interface further. I think we probably don’t want to use the custom component’s title, as the inner component will probably expose this itself - using it for the wrapper too would lead to duplicated titles, which would be confusing.

The rest of the team is on holiday this week so it’ll be a few days before anyone is able to review my work. If you have any further thoughts in the meantime I’d be interested to hear them.

Thanks again!

Triggered automatically is a good choice

If it’s true you are a button and you expect juce to do button things for you. Just add an accessible name and voila

If it’s false then you handle your own events and so have to handle your own accesibility

I bet that would work

Btw the feedback we are getting on juce accesibility from our early testers has been really good (and I learned a lot trying to make the synth have basic access). Thanks so much for making this a priority feature in juce!

Thanks for your input on this issue. I’ve pushed a fix here:

Hopefully the new behaviour is intuitive. If you run into any further problems, please let me know.

Oh thanks! I’ll pull this and others after our release.
Curious any ETA on a 6.1.5? We could jump to that rather than 6.1.2 + patches which we are using for release this week.