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.

Sign up for GitHub

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

Jump to bottom

Implement "Recently Used Boards" Menu #10816

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Implement "Recently Used Boards" Menu #10816

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7152b2
Implement "Recently Used Boards" Menu
NPoole Sep 29, 2020
2acc1f7
Suggested Changes from PR 10816
NPoole Oct 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 96 additions & 1 deletion app/src/processing/app/Base.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,13 @@ public class Base {
private List<JMenu> boardsCustomMenus;
private List<JMenuItem> programmerMenus;

// these variables help rebuild the "recently used boards"
// menu on board selection
private HashMap<String, JRadioButtonMenuItem> recentBoardItems;
private List<JRadioButtonMenuItem> recentBoardsToClear = new LinkedList<>();;
private JMenu boardMenu;
private int recentBoardsJMenuIndex;

private PdeKeywords pdeKeywords;
private final List<JMenuItem> recentSketchesMenuItems = new LinkedList<>();

Expand Down Expand Up @@ -1360,6 +1367,39 @@ public void onBoardOrPortChange() {
}
}

// Update recent boards list in preferences
List<String> newRecentBoardIds = new ArrayList<String>();
String currentBoard = PreferencesData.get("board");
for (String recentBoard : PreferencesData.getCollection("recent.boards")){
if (!recentBoard.equals(currentBoard)) {
newRecentBoardIds.add(recentBoard);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't this just use Collection.remove to remove currentBoard rather than manually looping? Might need to make a copy of the collection first, though just modifying the existing collection might be even shorter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or maybe this would be a good usecase for this new java streaming collection API, where you could do something like "get the current list, filter out the selected board, prepend the current board, and limit to n elements". Not sure about exact syntax, but I think this could very well end up concise and readable.

Copy link
Author

Choose a reason for hiding this comment

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

I'll look into that. I'm actually not that familiar with Java, this was my first crack at it. I suspect there are a lot of optimizations to do with collections since I mostly treated them like arrays.

Copy link
Author

Choose a reason for hiding this comment

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

I can't figure out the optimization for this without breaking it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I implemented this in my branch: matthijskooijman@818c3fe

newRecentBoardIds.add(0, currentBoard);

int numBoards = 0;

if (PreferencesData.has("recent.num_boards")) {
numBoards = PreferencesData.getInteger("recent.num_boards");
}
Comment on lines +1380 to +1384
Copy link
Collaborator

Choose a reason for hiding this comment

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

getInteger can take a default argument, so this can be:

Suggested change
int numBoards = 0;
if (PreferencesData.has("recent.num_boards")) {
numBoards = PreferencesData.getInteger("recent.num_boards");
}
int numBoards = PreferencesData.getInteger("recent.num_boards", 0);

But, given what I say about the default preferences.txt in another comment, I think the default can even be left out:

Suggested change
int numBoards = 0;
if (PreferencesData.has("recent.num_boards")) {
numBoards = PreferencesData.getInteger("recent.num_boards");
}
int numBoards = PreferencesData.getInteger("recent.num_boards");

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did this in my branch, as part of some other cleanup.


while (newRecentBoardIds.size() > numBoards) {
newRecentBoardIds.remove(newRecentBoardIds.size() - 1);
}
NPoole marked this conversation as resolved.
Show resolved Hide resolved
PreferencesData.setCollection("recent.boards", newRecentBoardIds);

// If recent.num_boards is 0, interpret this as the feature
// being turned off. There's no need to rebuild the menu
// because it will be hidden
if (numBoards > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this if could be broader: If numBoards == 0, then there's no point to even set up, add to and prune the list at all. It might still be needed to create an empty list to keep other code happy, but maybe not even.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In my branch, I ended up just removing this if instead, since the code should produce the right result in that case anyway (it is slightly slower, but having this disabled is a corner case that should not really be optimized for).

try {
rebuildRecentBoardsList();
} catch (Exception e) {
//TODO show error
e.printStackTrace();
}
}

// Update editors status bar
for (Editor editor : editors) {
editor.onBoardOrPortChange();
Expand Down Expand Up @@ -1433,9 +1473,10 @@ protected void onIndexesUpdated() throws Exception {

public void rebuildBoardsMenu() throws Exception {
boardsCustomMenus = new LinkedList<>();
recentBoardItems = new HashMap<String, JRadioButtonMenuItem>();

// The first custom menu is the "Board" selection submenu
JMenu boardMenu = new JMenu(tr("Board"));
boardMenu = new JMenu(tr("Board"));
boardMenu.putClientProperty("removeOnWindowDeactivation", true);
MenuScroller.setScrollerFor(boardMenu).setTopFixedCount(1);

Expand All @@ -1458,12 +1499,30 @@ public void actionPerformed(ActionEvent actionevent) {
}));
boardsCustomMenus.add(boardMenu);

// Insert recently used boards menu and remember index for insertion later
// Check if the field exists, in case preferences got lost
if (!PreferencesData.has("recent.num_boards")){
// (default to 5)
PreferencesData.setInteger("recent.num_boards", 5);
}
Comment on lines +1503 to +1507
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that setting default values for preferences like this is not normally done, instead it should be added to build/shared/lib/preferences.txt, which is always loaded before the user's preferences.txt IIRC, so that sets the default value for when the user's file does not contain the preference (which is then saved to the user's preferences.txt on quit).

So this could just be removed.

Suggested change
// Check if the field exists, in case preferences got lost
if (!PreferencesData.has("recent.num_boards")){
// (default to 5)
PreferencesData.setInteger("recent.num_boards", 5);
}

if (PreferencesData.getInteger("recent.num_boards") > 0) {
// Insert menu label
boardMenu.add(new JSeparator());
JMenuItem label = new JMenuItem(tr("Recently Used Boards"));
NPoole marked this conversation as resolved.
Show resolved Hide resolved
label.setEnabled(false);
boardMenu.add(label);
recentBoardsJMenuIndex = boardMenu.getItemCount();
}

// If there are no platforms installed we are done
if (BaseNoGui.packages.size() == 0)
return;

// Separate "Install boards..." command from installed boards
boardMenu.add(new JSeparator());
JMenuItem label = new JMenuItem(tr("All Installed Boards"));
label.setEnabled(false);
boardMenu.add(label);

// Generate custom menus for all platforms
for (TargetPackage targetPackage : BaseNoGui.packages.values()) {
Expand Down Expand Up @@ -1555,6 +1614,25 @@ private String getPlatformUniqueId(TargetPlatform platform) {
return platform.getId() + "_" + platform.getFolder();
}

// clear the previous menu items from the "recently used boards"
// menu and repopulate with updated items
private void rebuildRecentBoardsList() throws Exception {
Collection<String> recentBoardIds = PreferencesData.getCollection("recent.boards");
String currentBoard = PreferencesData.get("board");
int idxAdv = 0;
for (JRadioButtonMenuItem itemToClear : recentBoardsToClear) {
boardMenu.remove(itemToClear);
}
recentBoardsToClear.clear();
for (String boardId : recentBoardIds) {
JRadioButtonMenuItem addItem = recentBoardItems.get(boardId);
boardMenu.add(addItem, recentBoardsJMenuIndex+idxAdv);
recentBoardsToClear.add(addItem);
addItem.setSelected(boardId.equals(currentBoard));
idxAdv++;
}
}

private JRadioButtonMenuItem createBoardMenusAndCustomMenus(
final List<JMenu> boardsCustomMenus, List<JMenuItem> menuItemsToClickAfterStartup,
Map<String, ButtonGroup> buttonGroupsMap,
Expand Down Expand Up @@ -1585,6 +1663,23 @@ public void actionPerformed(ActionEvent actionevent) {

JRadioButtonMenuItem item = new JRadioButtonMenuItem(action);

// create an action for the "recent boards" copy of this menu item
// which clicks the original menu item
Action actionClone = new AbstractAction(board.getName()) {
public void actionPerformed(ActionEvent actionevent) {
item.setSelected(true);
item.getAction().actionPerformed(new ActionEvent(this, -1, ""));
}
};

// No need to hog memory if recent boards feature is turned off
if (PreferencesData.getInteger("recent.num_boards") > 0) {
// create a menu item for the "recent boards" menu
JRadioButtonMenuItem itemClone = new JRadioButtonMenuItem(actionClone);
// populate list of menuitem copies
recentBoardItems.put(boardId, itemClone);
}

if (selBoard.equals(boardId) && selPackage.equals(packageName)
&& selPlatform.equals(platformName)) {
menuItemsToClickAfterStartup.add(item);
Expand Down