Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Divide board menu into submenus #9238

Merged
merged 6 commits into from
Mar 25, 2020

Conversation

matthijskooijman
Copy link
Collaborator

When multiple platforms (board packages) are installed, the boards menu quickly becomes very long and hard to use. This PR changes the menu to have a single submenu for each installed platform, under which the boards for that platform are listed. This makes the list a lot faster to navigate:

Screenshot from 2019-09-19 17-00-33

When there is just a single platform installed (e.g. on the default install), submenus do not really add anything, so the board menu still lists the boards directly as before (the only change is that the platform name is no longer shown at the top of the menu):

Screenshot from 2019-09-19 16-56-58

I think there has been some discussion about this in the past (see #8858, #1986 is also somewhat related). This also relates to keeping a recently-used-boards list (PR at #8607), either that or this PR might need some refactoring if both will be merged (but I needed this feature myself, so I thought I might as well share it here as well).

@facchinm
Copy link
Member

I tested the patch and have one major concern; when you select a board, it doesn't appear anymore after "Board" label, just on the bottom right.

@matthijskooijman
Copy link
Collaborator Author

I tested the patch and have one major concern; when you select a board, it doesn't appear anymore after "Board" label, just on the bottom right.

Good point. It took me a while to find where this was done (ended up grepping for ": \" to find it...), it is a bit obscure code (IMHO it might be worth refactoring at some point to be a bit more explicit and less magical, but some refactoring might be good in all this menu code...).

In any case, I fixed the code to support nested menus now, I added a new commit for that before the existing commits (to make sure it is never broken in the history. The two existing commits are unchanged).

There is a small behavioral change (see commit message: e5551bf), removing a check that seems unneeded. @PaulStoffregen, you originally added this code. Do you happen to remember why this if is needed (i.e. when item.getText() could be null):

sel = item.getText();
if (sel != null) break;

@datenheim
Copy link

white_check_mark Build completed.
Please test this code using one of the following:

arrow_down http:https://downloads.arduino.cc/javaide/pull_requests/arduino-PR-9238-BUILD-892-windows.zip

Tested on Windows 7-64 Bit, works like a charm. Hope it makes it to the release soon :)

This in fact solves half of my feature request #8858.

Thank you.

@ladyada
Copy link

ladyada commented Sep 19, 2019

sending good vibes to this PR - we have to QA against so many packages it takes 15 seconds to scroll thru them all :D this is gonna be awesome!

@datenheim
Copy link

... and get even more useful when combined with the recently used boards #8607 which made it to the current beta already. Both features might be a good add to the insert library menu also.

@datenheim
Copy link

I tested the patch and have one major concern; when you select a board, it doesn't appear anymore after "Board" label, just on the bottom right.

@facchinm Indeed correct, this is a no-go. Overlooked that because I was so happy about the split board menu :(

@facchinm
Copy link
Member

@matthijskooijman did you forget to push the fix commit? 🙂

@matthijskooijman
Copy link
Collaborator Author

@matthijskooijman did you forget to push the fix commit? slightly_smiling_face

I was sure I pushed it, turns out I accidentally pushed to this repository rather than my own fork, w00ps. Anyway, fixed now, the extra fix commit is now added to this PR :-)

@facchinm
Copy link
Member

@tigoe @Giorgio-Olivero what do you think about this? Do you think it will improve the usability even for people with 2/3 cores installed?

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019

I like the concept, there are a couple of implementation issues I noticed:

When using the menus with VoiceOver on MacOS, there's a weird behavior that happens when you back out of a menu. Normally with VO, you hold the ctrl-option keys and navigate menus with the arrows. The main menu drops entirely rather than going back up one sub-menu. I think it'd be pretty common to browse the menus up and down a bit looking for your board, so making the arrow keys do the same traversing of the sub-menus that you'd do with a mouse would be appropriate, I think.

It brings up another issue I've seen with students and the Boards menu: most casual users and beginners don't know the architectures, they product lines. So they'd expect a Nano Every, for example, to be next to a Nano 33 IoT or BLE. Same with the Adafruit Feathers, the ESPs, etc. Sometimes that happens by virtue of being the same processor architecture, but often not. It would be handy to be able to sort the Boards submenus by product family or alphabetically, not just by architecture.

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019

Here's a test on that VoiceOver issue:

  • MacOS Mojave, turn on VO using command-F5
  • press ctrl-option-M to turn on voiceOver
  • press right arrow 5 times to get to tools menu
  • press down arrow until you're on Boards
  • press right arrow to open submenu
  • press down arrow to get to the Arduino SAMD boards
  • press right arrow to scroll through them
  • press left arrow to go up a submenu

At this last step, the entire menu closes on some cases without selecting a board. Doesn't seem to be all cases, though, and I can't identify what the connection between them is yet.

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019

@facchinm I also wanted to check your question of how it would be for users with only 2-3 cores. So I removed my Arduino15 folder and restarted. The default install only has the AVR boards, so the menu looks just like it does currently. When you add a core, it groups them by submenu appropriately, and seems convenient enough. So I'd say it works okay for both beginners and power users, once the glitch with voiceOver is solved.

I'd still add the "sort by..." option, but that should properly be a separate issue.

@matthijskooijman
Copy link
Collaborator Author

When using the menus with VoiceOver on MacOS,

Is this specific to using VO? AFAICS, VO means that OSX will read out selected items to you while navigating by keyboard. But you can navigate the menus already without VO, I think? Does it then also "break"? Or does VO actually change the way keyboard navigation works (I'm not a Mac user, so I'm not quite familiar with this).

Does it work properly with other menus inside the IDE? I'm wondering what would be different, since we essentially just create some nested JMenu/JMenuItem instances and let java handle the rest, so I wonder if this might just be the way Java handles this and whether there is anything we can do to influence how this works...

It brings up another issue I've seen with students and the Boards menu: most casual users and beginners don't know the architectures, they product lines. So they'd expect a Nano Every, for example, to be next to a Nano 33 IoT or BLE. Same with the Adafruit Feathers, the ESPs, etc. Sometimes that happens by virtue of being the same processor architecture, but often not. It would be handy to be able to sort the Boards submenus by product family or alphabetically, not just by architecture.

With "Sort", do you then mean changing the order of items? Or also changing the grouping in submenus? Grouping of boards seems to have been tried at some point in #120, however it seems that the custom menus have been used instead to solve that particular usecase (e.g. grouping Pro Mini 8Mhz and Pro Mini 16Mhz together).

To really make finding boards easy, one could imagine having a standalone board browser window, where you could switch between different orderings and groupings and filter by name, etc. but that is not so suitable for inside a drop-down menu. And as you said, I think this is really a separate thing that might need to be discussed (and implemented) separately later.

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019

The submenu and arrow issue seems to be the same when navigating with arrows without voice over on. On Windows, you could probably see the same with a screen reader turned on there. NVDA or JAWS would be good tests of it, but I don't have a Windows machine at home at the moment.

@matthijskooijman
Copy link
Collaborator Author

The submenu and arrow issue seems to be the same when navigating with arrows without voice over on. On Windows, you could probably see the same with a screen reader turned on there.

Huh? You're saying you do not need Voice Over to reproduce this problem, so why would one need a screenreader on Windows? Or did I misunderstand?

NVDA or JAWS would be good tests of it, but I don't have a Windows machine at home at the moment.

I don't have Windows at hand either, nor a screenreader.

I did test on Linux (Gnome 3), without a screenreader, and there keyboard navigation works as expected (press alt-T to activate the tools menu, navigate into the subsubmenu, press left and only the innermost menu closes).

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019

I think it should be tested on the other OSes, and with screen readers, before merging. The fact that I got unexpected behavior means others might too.

@matthijskooijman
Copy link
Collaborator Author

I just tested the build on Windows 10 (without screenreader, I have no admin access here to install anything) and the arrow-key-behaviour is also ok.

@matthijskooijman
Copy link
Collaborator Author

@tigoe, Another question: Is this unwanted arrow-key behaviour specific to the (extra nested) boards menu? Or did it occur already with the unmodified releases builds of Arduino? In the latter case, this might be a separate issue (though this PR does make the issue more pronounced because of the additional layer of nested menus, of course)?

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019 via email

@matthijskooijman
Copy link
Collaborator Author

That's good. Windows 10 has a built-in screen reader called Narrator. Give
that a try.

Ah, thanks. Just tried that, and could not really get Narrator to speak out anything from the Arduino interface, it seems. Also, unlike your voice-over thing, I think there is no hotkey to activate Narrator to navigate through the menubar. I could not reproduce your problem with Narrator running.

However, I did notice this:

  • If you open up the tools menu
  • Then move the mouse over the "Board" item
  • Then close the tools menu, and reopen it using alt+T
  • Then the mouse is still over the board item, so the board submenu opens, but no item is selected yet
  • If you now press right-arrow, instead of entering the submenu, it navigates from Tools to Help in the top menu bar. Pressing down or up rather than right does enter the menu as expected (and then pressing left leaves it as expected).

Maybe this is also what you've been seeing? Or perhaps the mouse position at least explains the erraticness of what you've been seeing? This behaviour happens here on Windows 10 both with or without Narrator, both with the 1.8.9 release and this PR build. It does not happen with menus in Chrome or regedit, so I suspect this might be specific to the Java implementation of menus.

@tigoe
Copy link
Member

tigoe commented Sep 20, 2019 via email

@matthijskooijman
Copy link
Collaborator Author

I've just did another force-push to remove an unrelated commit about String examples I accidentally added, and to add a fixup commit to slightly simplify and optimize some code and fix an unused variable warning pointed out by Codacy at the same time.

The fixup commit should be squashed before merging this commit.

@tigoe
Copy link
Member

tigoe commented Sep 23, 2019

I ran your latest build this morning on MacOS and tested with VoiceOver again. Couldn't replicate the problem I was hitting last week. Either your cleanup fixed it or I was imagining things in my earlier test, but either way we can close that thread.

The Adafruit Circuit Playground Express showed up in the Arduino SAMD submenu, whereas the rest of the Adafruit boards showed in their own Adafruit SAMD submenu. I was a little confused by that, but it's not a show stopper for me.

@ladyada
Copy link

ladyada commented Sep 23, 2019

@tigoe that's correct, Adafruit Circuit Playground Express should appear under Arduino SAMD And Adafruit Circuit Playground will be under Arduino AVR

@bpeschier
Copy link

Here's a test on that VoiceOver issue:

  • MacOS Mojave, turn on VO using command-F5
  • press ctrl-option-M to turn on voiceOver
  • press right arrow 5 times to get to tools menu
  • press down arrow until you're on Boards
  • press right arrow to open submenu
  • press down arrow to get to the Arduino SAMD boards
  • press right arrow to scroll through them
  • press left arrow to go up a submenu

At this last step, the entire menu closes on some cases without selecting a board. Doesn't seem to be all cases, though, and I can't identify what the connection between them is yet.

Hi, I tested this with the version you are referencing here and the later version and could not get it replicated with either. Only the submenu gets closed, which is the expected behaviour.

@tigoe
Copy link
Member

tigoe commented Sep 23, 2019

Good, thanks all for the confirmations. From there, I will defer to @facchinm.

Previously, the Tools->Boards menu was one long list, divided into
different platforms by (unselectable) headers. When more than one or two
platforms were installed, this quickly results in a very long list of
boards that is hard to navigate.

This commit changes the board menu to have a submenu for each platform,
where each submenu contains just the boards for that platform.

Note that this first keeps a list of board items and then adds those to
the boards menu later. This could have been done directly, but the
intermediate list makes it easier to special-case single platform
installations, as well as sort the list in subsequent commits.

This fixes part of arduino#8858.
When just one platform is installed, it does not make much sense to use
a submenu, so just add the boards directly under the boards menu as
before.
This sorts the board submenus themselves, based on the displayed name.
This does not change the ordering of board items within these submenus
(which uses the order from boards.txt).
@cmaglie
Copy link
Member

cmaglie commented Mar 25, 2020

Rebased to master and added a commit: 747ce01

Use un-translated labels to sort boards submenus

Otherwise it may happen some weird sorting when untraslated and
translated labels are sorted together:

    Arduino megaAVR Boards
    Arduino nRF52 Board
    ESP32 Arduino
    ESP8266 Modules
    Schede Arduino AVR   <-- the localized string falls to the bottom

all the Arduino boards should be packed together, so it's better to not
use localization at all while sorting. After this patch it will look like

    Schede Arduino AVR
    Arduino megaAVR Boards
    Arduino nRF52 Board
    ESP32 Arduino
    ESP8266 Modules

@cmaglie cmaglie added this to the Release 1.8.13 milestone Mar 25, 2020
@matthijskooijman
Copy link
Collaborator Author

Hm, is it not actually unexpected if the sorting does not follow the displayed strings? This might end up with grouping related platforms based on their untranslated names, but this might be unclear to a user.

I was going to say that sorting on translated strings should work fine if all platforms are consistently translated, but I think there is currently no way for third-party cores to supply any translations at all, so the only translations available are those which are supplied with the IDE (which might even be limited to the AVR core). Would it not make sense to simply not translate the platform names at all, so at least all are treated equally?

@cmaglie
Copy link
Member

cmaglie commented Mar 25, 2020

The sorting should be based on the vendor/architecture and not on the word "Boards". What happens is that in english we have:

Arduino AVR Boards

since the word "Boards" is at the end of the string it basically doesn't influence the sorting but when we go to italian the word "Schede" is placed at the beginning of the string:

Schede Arduino AVR

So sorting in english happens to be correct but it may not be the same in other languages.
Another thing to consider is that some platforms doesn't have the word "Boards" at all in their name, so for example ESP32 Arduino will stay the same in every language and it will be presented before Schede Arduino AVR in italian. IMHO this is even more unexpected...

but I think there is currently no way for third-party cores to supply any translations at all

This is a good point, I agree, better to remove the translation here.

@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
@arduino arduino deleted a comment from ArduinoBot Mar 25, 2020
Otherwise it may happen some weird sorting when untraslated and
translated labels are sorted together:

    Arduino megaAVR Boards
    Arduino nRF52 Board
    ESP32 Arduino
    ESP8266 Modules
    Schede Arduino AVR   <-- the localized string falls to the bottom

Also there is no way for 3rd party boards developers to actually provide
a translation, so let's just remove them.
Copy link
Collaborator Author

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmaglie cmaglie merged commit a1e43ce into arduino:master Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants