Follow up to work that added refresh buttons to editors from the Variable Explorer

[ccordoba12]

This is a series of tasks that I think we should address after merging PR #21312:

• Make editors inherit from SpyderWidgetMixin and use its API to build actions, menus and toolbars. I think that's important because those widgets are quite complex now, so if we plan to add more functionality to them in the future, it'd be nice if we could do it in a more structured way. And using that API could help with that.
• Review the Dataframe editor toolbar and context menu ordering. I think that toolbar could see a bit love to look a bit better. We can talk about it here.
• Remove checkboxes in Dataframe editor and add those as buttons to its toolbar.
• Fix margin and alignment of buttons in all editors. Specifically:
  • There's too much margin between the content area and the buttons.
  • Buttons are not aligned to the right of the content area, which makes the layout look odd.
• Make Object explorer toolbar look similar to the toolbars shown in the other editors.

[ccordoba12]

@jitseniesen, could you take care of this one?

[jitseniesen]

@jitseniesen, could you take care of this one?

I will have a look at it.

[jitseniesen]

@ccordoba12 I thought a bit about this and there is a fair amount of stuff to talk about.

Make editors inherit from SpyderWidgetMixin and use its API to build actions, menus and toolbars. I think that's important because those widgets are quite complex now, so if we plan to add more functionality to them in the future, it'd be nice if we could do it in a more structured way. And using that API could help with that.

The API of SpyderWidgetMixin stores the menus and toolbars in a global registry. I don't understand what that is good for. I'm afraid that this is actually harmful, because it means that there will always be a reference to the menus and toolbars so they will never be garbage collected, which probably means that the editors themselves and the data stored in them will also not be garbage collected (though I did not look in the details).

Can somebody explain what the reason is for storing everything in a global registry as this is something that I have wondered about before.

I actually thought about having all the editors inherit from a base class. Currently the implementations are all different and this would unify and simplify the code, and longer term allow us to add an API for plugins to add editors for other types. But I decided that this would be too much work for 6.0.

• Review the Dataframe editor toolbar and context menu ordering. I think that toolbar could see a bit love to look a bit better. We can talk about it here.

• Remove checkboxes in Dataframe editor and add those as buttons to its toolbar.

Currently, the toolbars and context menus of the different editors look as follows:

• Array editor:
  • Context menu: Copy
  • Toolbar: Copy, Edit, Set format, Resize columns, Toggle background, Refresh
• Collections editor:
  • Context menu: Edit, Copy, Paste, Rename, Remove, (Save array), separator, Insert, (Insert above), (Insert below), Duplicate, separator, View with object explorer, (Plot), (Histogram), (Show image), separator, Resize rows, Resize columns
  • Toolbar: same as context menu, and also Refresh at the end
  • The items between parentheses are only shown for suitable variable types
• Dataframe editor:
  • Context menu: Edit, Copy, Remove row, Remove column, separator, Insert above, Insert below, Insert before, Insert after, Duplicate row, Duplicate column, separator, Convert to (bool, complex, int, float, str), separator, Resize row, Resize column, separator, Refresh
  • Toolbar: same as context menu, except for "Convert to"
  • Buttons at the bottom: Set format, Resize, Toggle background, Toggle column min/max
• Object explorer:
  • No context menu
  • Toolbar: Show callables, Show special attributes, Refresh, Options menu (for toggling visibility of columns)

So there is definitely some room for improvements. Any specific suggestions?

• Fix margin and alignment of buttons in all editors. Specifically:

  • There's too much margin between the content area and the buttons.
  • Buttons are not aligned to the right of the content area, which makes the layout look odd.

I assume that you are talking about the buttons at the bottom ("Save and close" and "Close"). I will copy the spacing from the Python Path dialog and see how that works out.

• Make Object explorer toolbar look similar to the toolbars shown in the other editors.

Okay.

[ccordoba12]

The API of SpyderWidgetMixin stores the menus and toolbars in a global registry. I don't understand what that is good for.

It's done to easily get access to a menu or toolbar by its id from any other plugin or widget that inherits from SpyderWidgetMixin.

I'm afraid that this is actually harmful, because it means that there will always be a reference to the menus and toolbars so they will never be garbage collected, which probably means that the editors themselves and the data stored in them will also not be garbage collected (though I did not look in the details).

I agree that for the Variable Explorer editors that's not going to allow Python to garbage collect them after they are closed. To solve this and keep using the SpyderWidgetMixin API, I propose to add a register kwarg to create_menu and create_toolbar to avoid registering them if requested.

I actually thought about having all the editors inherit from a base class. Currently the implementations are all different and this would unify and simplify the code, and longer term allow us to add an API for plugins to add editors for other types. But I decided that this would be too much work for 6.0.

Ok, no problem.

So there is definitely some room for improvements. Any specific suggestions?

I think @CAM-Gerlach can help you to review and decide on a nice ordering for actions in toolbars and menus, which could involve adding separators for the Dataframe toolbar because it has too many actions.

So, please talk to him about that.

I assume that you are talking about the buttons at the bottom ("Save and close" and "Close"). I will copy the spacing from the Python Path dialog and see how that works out.

Correct.

[jitseniesen]

• There's too much margin between the content area and the buttons.

@ccordoba12 I see in pathmanager.py that you use addSpacing((-1 if MAC else 2) * AppStyle.MarginSize) for the spacing between the content area and the buttons at the bottom. Is that something to use here too? I don't understand the correction for Mac, but I don't have one to test with.

[ccordoba12]

Is that something to use here too?

Yes, I think that's a good idea.

I don't understand the correction for Mac, but I don't have one to test with.

Mac adds by default too much space between the content area and the buttons below it. So, we don't need to add more space (as it's necessary on Windows and Linux, hence the 2 * AppStyle.MarginSize), but remove a bit of it (that's the reason for the -1 * AppStyle.MarginSize there).

[jitseniesen]

Make editors inherit from SpyderWidgetMixin and use its API to build actions, menus and toolbars

@ccordoba12 I did not notice this before but SpyderWidgetMixin does not have an API for toolbars because it does not inherit from SpyderToolbarMixin:

spyder/spyder/api/widgets/mixins.py

Lines 618 to 625 in df5c5bd

	class SpyderWidgetMixin(SpyderActionMixin, SpyderMenuMixin,
	SpyderConfigurationObserver, SpyderToolButtonMixin):
	"""
	Basic functionality for all Spyder widgets and Qt items.
	This mixin does not include toolbar handling as that is limited to the
	application with the coreui plugin or the PluginMainWidget for dockable
	plugins.

I am going to change this and add SpyderToolbarMixin as a base class of SpyderWidgetMixin; please let me know if that is wrong.

[ccordoba12]

I am going to change this and add SpyderToolbarMixin as a base class of SpyderWidgetMixin;

@jitseniesen, that's a good idea, thanks for thinking about it.

please let me know if that is wrong.

I quickly looked at the other classes from which SpyderWidgetMixin inherits and there shouldn't be problems with adding SpyderToolbarMixin to it as another parent.

[CAM-Gerlach]

General thinking

Categories

• Context menu for actions that depend on the currently-selected cell, row or column, etc.
• Toolbar for non-context-specific actions, toggleables and common actions not easily accessible via other existing means (keyboard, shortcuts, etc)
  • Left of the toolbar for actions that affect the actual content
  • Right of the toolbar for special actions that just change the display of the content
• Options ("hamburger") menu for uncommon actions and infrequently-changed options and settings

Priorities

1. Ensure each menu has the options appropriate for its purpose
2. Reduce overlap of items between the three, where justified by the above
3. Attempt to better balance the number of entries between menus, where practical

List of recommended changes

• Since it is the only button present for every editor and is probably one of the most common buttons users will click, as well as being something that applies to the whole object and always enabled rather than a specific item and disabled iwthout a selection, I suggest moving the Refresh button to float the right of the toolbar, directly to the left of the Options menu
• Place always-enabled global actions before (and in a separate group from) context-specific, default-disabled local actions in toolbars

Namespace browser (Variable Explorer)

• Float Search, Filter and Refresh to right, next to "Options", and re-order
• Move Filter options to under Filter button (as menu)

Array editor

• Add Edit to context menu (after Copy), since it applies to an individual item just like Copy
• Remove Copy and Edit from toolbar, since they are context-specific, already in the context menu and can be easily triggered via existing means (mouse clicks and keyboard shortcuts)

Collections editor

• Remove Resize Rows and Resize Columns from context menu as they are not context-specific are present on the toolbar
• Change Save array to just Save, to match the wording of others and avoid confusion in nested arrays
• Change context menu ordering to Copy Paste Rename Edit [Save] | Insert above Insert below Duplicate Remove | ... for more logical grouping and ordering
• Remove highly context-sensitive, basic/typically expected and already easily triggerable (keyboard shortcut, mouse click) items in first group from toolbar (copy, paste, rename and edit)
• Re-order toolbar to place the global, always-enabled toggles (Refresh, Resize rows, Resize columns) first, and then follow the revised order of the context menu

DataFrame editor

• Similar changes as in collections editor
• Remove Refresh inexplicably present in context mneu

Object Explorer

• No additional changes (besides moving Refresh)

Final recommended arrangement

Namespace browser (global Variable Explorer pane)

• Toolbar: Import Save data Save data as Remove all variables <--> Search Filter Refresh Options
  • Filter (menu): Exclude options
• Options menu: Show array min/max plus pane-level options

Array editor

• Context menu: Copy Edit
• Toolbar: Resize columns <--> Refresh
• Options menu: Background color Set format

Collections editor

• Context menu: Copy Paste Rename Edit [Save] | Insert above Insert below Duplicate Remove | View with object explorer [Plot] [Histogram] [Show image]
• Toolbar: Refresh Resize rows Resize columns | Insert above Insert below Duplicate Remove | View with object explorer [Plot] [Histogram] [Show image]

DataFrame editor

• Context menu: Copy Edit | Insert above, Insert below, Insert before Insert after Duplicate row Duplicate column Remove Row Remove Column | Convert to (bool, complex, int, float, str)
• Toolbar: Resize rows Resize columns | Insert above, Insert below, Insert before Insert after Duplicate row Duplicate column Remove Row Remove Column <--> Refresh
• Options menu: Background color Column Min/max Set format

Object explorer

• No context menu
• Toolbar: Show callables Show special attributes <--> Refresh
• Options menu: Column visibility toggles

[jitseniesen]

@CAM-Gerlach Thanks for your clear and well-reasoned suggestions. They all make sense to me so I implemented them in the PR. There are a couple of further details (which I perhaps should have discussed before submitting the PR)

Firstly, the dataframe editor currently has four buttons in the bottom left of the dialog window: Format, Resize, (toggle) Background color and (toggle) Column min/max. The first is already in the toolbar. Consistency with the array editor means that the second and third should also go in the toolbar, so that it becomes: Refresh Set format Resize rows Resize columns Toggle background Insert above Insert below Insert before Insert after Duplicate row Duplicate column Remove Row Remove Column (the bold items are added compared to your comment).

I did not know what to do with the last button, Column min/max. In the end, I added an option menu with just that one item, because I think it is rarely used and I could not think of a good icon for it. The argument against is that we now have an option menu with just one item, which looks a bit silly.

Secondly, you indicated separators in the toolbar. Are these supposed to be visible in the UI? I don't know where in Spyder we use separators in the toolbar.

Thirdly, in the Variable Explorer pane itself, the Refresh button is not at the front of the toolbar. Should I move it there?

[CAM-Gerlach]

BTW, for others following, I had updated the above comment to incorporate @jitseniesen 's feedback and further adjustments following discussion with @ccordoba12 , @jitseniesen and @conradolandia . Specifically, here's a changelog:

• Added the namespace browser (Variable Explorer main pane) and its recommended menus and changes
• Moved the Refresh option, and other display-related options, to float to the right side of the toolbar instead of being on the left, to be more consistent with general UI expectations and with the top-level namespace browser (Variable Explorer)
• Added and adjusted the positions of the Format, Background color, Column min max and resize options in the array editor and other relevant toolbars