Skip to content

Enhance Help Menu for Better Functionality Discoverability #314

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
81 changes: 76 additions & 5 deletions mne_qt_browser/_pg_figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
from pathlib import Path

import numpy as np
from PyQt5.QtTest import QTest
from PyQt5.QtWidgets import QMenuBar

try:
from qtpy.QtCore import Qt
Expand Down Expand Up @@ -4017,10 +4019,6 @@ def __init__(self, **kwargs):
asettings.triggered.connect(self._toggle_settings_fig)
self.mne.toolbar.addAction(asettings)

ahelp = QAction(self._qicon("help"), "Help", parent=self)
ahelp.triggered.connect(self._toggle_help_fig)
self.mne.toolbar.addAction(ahelp)

# Set Start-Range (after all necessary elements are initialized)
self.mne.plt.setXRange(
self.mne.t_start, self.mne.t_start + self.mne.duration, padding=0
Expand Down Expand Up @@ -4234,6 +4232,80 @@ def __init__(self, **kwargs):
# disable histogram of epoch PTP amplitude
del self.mne.keyboard_shortcuts["h"]

self.create_menus()

def create_menus(self):
"""Creates and returns the application's menu bar with properly aligned shortcuts."""
if not self.menuBar():
self.setMenuBar(QMenuBar(self))
menu_bar = self.menuBar()
mne_python = menu_bar.addMenu("MNE-Python")
view_menu = menu_bar.addMenu("View")
help_menu = menu_bar.addMenu("Help")
scroll_menu = view_menu.addMenu("Scroll")

def add_menu_action(menu, description, shortcut):
widget = QWidget()
layout = QHBoxLayout(widget)
layout.setContentsMargins(10, 2, 10, 2)
Copy link
Member

Choose a reason for hiding this comment

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

... this could probably be tweaked rather than using spaces below to fix the spacing.


desc_label = QLabel(description)
shortcut_label = QLabel(shortcut)

layout.addWidget(desc_label)
layout.addStretch()
layout.addWidget(shortcut_label)

action = QWidgetAction(self)
action.setDefaultWidget(widget)
menu.addAction(action)

shortcut_data = {
mne_python: [
("Create annotation ", "Click + Drag"),
("Mark/unmark bad channel ", "Left-click channel"),
("Mark/unmark bad segment ", "Left-click data"),
("Toggle Annotation Tool / Visibility ", "A"),
("Toggle Butterfly ", "B"),
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a good start. A few ideas:

  1. From a UI perspective, a clickable menu item should initiate an action. Some of these can't, such as "Create Annotation". These should at least be "disabled" (i.e., still visible but not clickable). Not 100% sure we want these in a menu but... maybe okay to leave them.
  2. Some of them when clicked should initiate the action, such as the "Toggle DC Correction" -- clicking it should actually toggle DC correction.

Also this code isn't quite DRY... every first option has a bunch of extra spaces at the end. (1) I don't think these should be needed, and (2) if they absolutely are, they could be added to add_menu_action rather than being in every one of these entries.

("Toggle DC Correction ", "D"),
("Toggle Events visibility ", "E"),
("Toggle Projection Figure / all projections ", "J"),
],
view_menu: [
("Add/remove channels ", "Shift + Click"),
("Decrease duration (¼ page) ", "Home"),
("Decrease Scale ", "-"),
("Decrease shown channels (1/10) ", "Page Up"),
("Increase duration (¼ page) ", "End"),
("Increase Scale ", "+ / ="),
("Increase shown channels (1/10) ", "Page Down"),
("Navigate time/channels ", "Scroll"),
("Open context menu ", "Right-click plot"),
("Toggle Antialiasing ", "L"),
("Toggle Overview Bar ", "O"),
("Toggle Time Format ", "T"),
("Toggle Scalebars ", "S"),
("Toggle Whitening ", "W"),
("Toggle Crosshair ", "X"),
("Toggle Zen Mode ", "Z"),
],
help_menu: [
("Show Help ", "?"),
("Toggle Fullscreen ", "F11"),
("Close ", "Escape"),
],
scroll_menu: [
("Scroll left (¼ page/full page) ", "← / →"),
("Scroll up (full page) ", "↑"),
("Scroll down (full page) ", "↓"),
],
}

for menu, items in shortcut_data.items():
for description, shortcut in items:
add_menu_action(menu, description, shortcut)
Copy link
Member

Choose a reason for hiding this comment

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

You should use the same shortcuts as defined for the browser window, e.g.,

    help_menu.addAction(
         'Show keyboard shortcuts',
         browser._toggle_help_fig,
         shortcut=browser.mne.keyboard_shortcuts['?']['qt_key']
     )

See #126

return menu_bar

def _hidpi_mkPen(self, *args, **kwargs):
kwargs["width"] = self._pixel_ratio * kwargs.get("width", 1.0)
return mkPen(*args, **kwargs)
Expand All @@ -4248,7 +4320,6 @@ def _add_scalebars(self):
range)
"""
self.mne.scalebars.clear()
# To keep order (np.unique sorts)
ordered_types = self.mne.ch_types[self.mne.ch_order]
unique_type_idxs = np.unique(ordered_types, return_index=True)[1]
ch_types_ordered = [ordered_types[idx] for idx in sorted(unique_type_idxs)]
Expand Down
31 changes: 0 additions & 31 deletions mne_qt_browser/tests/test_pg_specific.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,27 +487,6 @@ def test_pg_settings_dialog(raw_orig, pg_backend):
assert ch_sens_spinbox.value() != orig_sens


def test_pg_help_dialog(raw_orig, pg_backend):
"""Test Settings Dialog toggle on/off for pyqtgraph-backend."""
fig = raw_orig.plot()
fig.test_mode = True
QTest.qWaitForWindowExposed(fig)
QTest.qWait(50)
assert fig.mne.fig_help is None
fig._fake_click_on_toolbar_action("Help", wait_after=500)
assert fig.mne.fig_help is not None
assert pg_backend._get_n_figs() == 2
fig._fake_click_on_toolbar_action("Help", wait_after=500)
assert fig.mne.fig_help is None
assert pg_backend._get_n_figs() == 1
fig._fake_click_on_toolbar_action("Help", wait_after=500)
assert fig.mne.fig_help is not None
assert pg_backend._get_n_figs() == 2
fig._fake_click_on_toolbar_action("Help", wait_after=500)
assert fig.mne.fig_help is None
assert pg_backend._get_n_figs() == 1


def test_pg_toolbar_time_plus_minus(raw_orig, pg_backend):
"""Test time controls."""
fig = raw_orig.plot()
Expand Down Expand Up @@ -672,16 +651,6 @@ def test_pg_toolbar_actions(raw_orig, pg_backend):
assert pg_backend._get_n_figs() == 3
fig._fake_click_on_toolbar_action("Settings", wait_after=100)
assert pg_backend._get_n_figs() == 2
fig._fake_click_on_toolbar_action("Help", wait_after=200)
assert pg_backend._get_n_figs() == 3
fig._fake_click_on_toolbar_action("Settings", wait_after=200)
assert pg_backend._get_n_figs() == 4
fig._fake_click_on_toolbar_action(SHOW_PROJECTORS, wait_after=200)
assert pg_backend._get_n_figs() == 3
fig._fake_click_on_toolbar_action("Settings", wait_after=100)
assert pg_backend._get_n_figs() == 2
fig._fake_click_on_toolbar_action("Help", wait_after=100)
assert pg_backend._get_n_figs() == 1


# LAB values taken from colorspacious on 2024/06/10
Expand Down