PR: Improve UI/UX of the Run plugin configuration widgets #22141

ccordoba12 · 2024-06-03T00:15:24Z

Description of Changes

The new architecture for the Run plugin (added in PR: Generalize Run plugin to support generic inputs and executors #17467) is quite powerful and flexible. However, it's not easy to understand how to make use of it.
This PR aims to improve that by
- Adding default execution parameters to our config system. That will show them in the Preferences page for Run, which will allow users to easily modify them. In addition, that page won't appear empty, which is a bit odd.
- Simplifying significantly the UI of the dialog that appears when you go to Run > Open run settings. In addition, that dialog was modified so that it allows to only set run settings per file (just as in Spyder 5). That makes it simpler to understand and leaves creating global execution configs to the Run entry in Preferences.
Add a new CollapsibleWidget widget, which is based on QCollpasible from SuperQt, and use it in RunDialog. That required increasing our minimal constraint on SuperQt to >=0.6.2.

Visual changes

Configuration per file

Before
After

Preferences page

Before
After

Dialog to create global run configurations

Before
After

Issue(s) Resolved

Fixes #21878
Fixes #20700
Fixes #20569

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @ccordoba12

- Instead, show label with the current file name. - That helps to decrease the complexity of this dialog.

This also helps to decrease the complexity of that dialog.

- The name of that config is the "Custom" string localized. - If the config is not named differently, additional customizations will also be saved in "Custom". - Also, only show global configs (i.e. those created in Preferences) or the ones that correspond to the file currently displayed in RunDialog.

- For the dialog: * Move configuration name to the beginning. * Simplify layout of extension and context comboboxes. * Show localized executor name in dialog's title. * Prevent saving configs without a name. - For the conf page: * Don't show configs set for files. * Code style improvements. - Also, remove several unused constants.

Also, disable delete and clone buttons if there's no index selected in RunParametersTableView and add a test for this new functionality.

- Also, improve the main label of its config page and fix the name of one of its actions.

Also add test to cover that case.

- This widget is based on superqt's QCollapsible widget. - It has several style customizations on top to match our style.

…ialog - Also, move QLineEdit to name the config to the top and show it inside a QGroupBox. - And move extra spacing between QGroupBoxes to the style of the Preferences dialog.

- IPython console group: Remove grid layout to give more space to introduce command line options. - External terminal: Change text of of several widgets to make options easier to understand. - Profiler: Minor inheritance problems.

…leWidget - We use PointingHandCursor for that, which will better indicate to users that the button is clickable. - That requires a more recent version of superqt, which exposes that button publicly.

- Correctly display the entries of its comboboxes now that they are instances of SpyderComboBox. - Fix error in Python 3.10+ when centering RunDialog. - Fix error in its config page when an executor doesn't have parameters for specific files.

This is done to follow the same convention present in the Profiler and External Terminal plugins.

This will give more space to users to enter them.

- Remove extra margins added by the config dialog style that this dialog belongs to. - Make the dialog fit exactly to the size of the widgets inside it. - Increase inner padding. - Remove unnecessary widget that acted as container for the others. - Reorganize code for clarity.

The previous solution was not correct because it cropped the title of those QGroupBoxes for some font types.

Those extensions are correctly added by other plugins.

…read

- That way those configurations will be displayed in the Run confpage, which will allow users to easily check what are the default parameters for each one. - Also, fix showing the last selected parameters for the file after opening the dialog.

- Simplify text for its opening label and buttons. - Add label to the left of the executor combobox to make clearer what it's about. - Don't show parameters table inside a QGroupBox to decrease its importance (they depend on the executor). - Add left and right horizontal margins to the parameters table to improve the table's left alignment.

- Show parameters name in the first column, then extension and finally context. - Reorganize parameters so that Python and IPython extensions are shown first and second by default. - Simplify column names. - If parameters are default, show a localized version of "Default" instead of its actual name. That's useful in case users switch between select different languages in the interface.

That way it'll be easier for users to understand that they can edit even the default configurations.

- Use localized version of "Default" string instead of the params name. - Don't allow to change the params name.

dalthviz · 2024-06-06T23:29:10Z

Gave this an initial local check on Windows and saw some behaviors/interactions that felt kind of strange (the last couple of things are more likely bugs):

Information tooltip refering to the Ok button at first glance make me think that it was refering to the Save globally button instead of the OK button at the bottom of the dialog itself. Not sure if it was discussed before, but now that I'm trying the dialog is kind of strange to have buttons in different places of the dialog: Also, the Delete button refers to a global delete of the config right?
Being able to trigger a file run config delete when I haven't save anything yet (even a confirmation dialog appears and then the run dialog gets closed):
Triggering a Save globally without entering a name/leaving the default one triggers a confirmation dialog before showing and red circle x over the config name lineedit:
Trying to save a new global config without a name changes the lineedit placeholder to a message indicating a name is needed. Maybe to make things more clear putting the red circle x over the lineedit:
Seems like trying to modify a global config from the run config dialog ends up deleting the global config?:

A traceback when using Reset changes and Edit selected buttons on the Run preference page:

Traceback (most recent call last):
  File "E:\Acer\Documentos\Spyder\Spyder otros\carlos\spyder\spyder\plugins\run\confpage.py", line 251, in <lambda>
    lambda checked: self.params_table.show_editor()
  File "E:\Acer\Documentos\Spyder\Spyder otros\carlos\spyder\spyder\plugins\run\confpage.py", line 111, in show_editor
    (extension, context, params) = model[index]
  File "E:\Acer\Documentos\Spyder\Spyder otros\carlos\spyder\spyder\plugins\run\models.py", line 540, in __getitem__
    tuple_index = self.params_index[index]
KeyError: -1

Other than that seems like things are working as expected but probably having more people testing this could be worthy (seems like there could be some interactions that I haven't checked yet 🤔)

jitseniesen · 2024-06-07T05:26:37Z

Just looking at the pictures, I would remove the heading "Runner settings" in the dialog windows. As a user, it is not clear why the working directory settings are separate from the runner settings. This will also simplify the dialog by removing one level of nesting.

- This helps to simplify the dialog and avoid some serious UX issues found in review. - Also, enable the delete button only for file configurations and fix tests that were affected by this change.

ccordoba12 · 2024-06-10T00:23:46Z

To @dalthviz:

Information tooltip refering to the Ok button at first glance make me think that it was refering to the Save globally button instead of the OK button at the bottom of the dialog itself. Not sure if it was discussed before, but now that I'm trying the dialog is kind of strange to have buttons in different places of the dialog

We discussed this with @conradolandia and agree with you. That's why I removed "Save globally" and moved the Delete button to the dialog button box at the bottom.

To help users understand where to set global configurations, I added a new entry called like that to the Run menu:

Also, the Delete button refers to a global delete of the config right?

No, it refers to deleting file configurations only. To clarify that, I changed the behavior of that button so that it's disabled for default or global configurations.

Being able to trigger a file run config delete when I haven't save anything yet (even a confirmation dialog appears and then the run dialog gets closed)

It should be fixed now by the change I mentioned above.

Triggering a Save globally without entering a name/leaving the default one triggers a confirmation dialog before showing and red circle x over the config name lineedit

It's no longer a problem.

Trying to save a new global config without a name changes the lineedit placeholder to a message indicating a name is needed. Maybe to make things more clear putting the red circle x over the lineedit

That's a great suggestion to make things consistent. I implemented it in one of my last commits.

Seems like trying to modify a global config from the run config dialog ends up deleting the global config?

It shouldn't be a problem anymore.

A traceback when using Reset changes and Edit selected buttons on the Run preference page

Thanks for noticing it. It should be fixed now.

To @jitseniesen:

Just looking at the pictures, I would remove the heading "Runner settings" in the dialog windows

Great suggestion! I did that in one of my last commits (see updated screenshots in the OP).

ccordoba12 · 2024-06-10T00:25:32Z

In addition to fixing the problems found in review, I took the opportunity to improve the design of the Run page in Preferences (see the updated OP).

dalthviz · 2024-06-10T22:14:49Z

Gave another check and I think the difference between the local vs global configs is clearer now 👍 However, there are a couple of things that seem like bugs/I'm still not sure about:

Cloning/duplicationg a global config and then removing the selected config removes all the created global configs:
Create a new local config, open the dialog again, try to modify the config without changing the name lineedit. The circular red x appears. Following that, try to set as preset configuration the default from the combobox (or use the reset button), still unable to close the dialog via the Ok button

Open the run dialog and choose a different preset or even just open and try to close it via the Ok button the dialog will be kept open due to the name:

So my guess is that it is not possible to override an existing local config? In fact, as things are right now, is not possible to select one of the already created configs/presets without creating a new config, right? If that is the case, probably more feedback about the error needs to be provided. Some ideas:

Add some sort of message somewhere indicating that a new name is needed
When showing the collapsible section, remove the text along side showing the circle red x and move focus to the line edit to point people not only that something is invalid but that they need to take action to overcome it

From the checks I have been doing, I would say that from the run dialog it would be nice to support the following operations:

Select a config from the presets and use it without having to do any modification (without having to deal with the Custom configuration section)
Be able to select a preset and change a specific config without having to create a new one
Be able to select a preset and use it as a base for a new config with a different name

Not sure how much of the these things are feasible/fall under the scope you had for this PR/the general functionality of the run dialog @ccordoba12 but sharing the ideas that came to mind while checking this

- Show error status icon when name is empty or already taken. - Disable changing the config name if it's a saved one. - Show the same messages in both ExecutionParametersDialog and RunDialog.

- Show current text on their tooltips and use icons instead. - That allows us to organize the buttons in a horizontal layout, which is more compact and lean. - For this it was useful to extend the create_button method of SpyderConfigPage to simplify the creation of buttons in confpages. - Also, fix extra padding in tooltips of SidebarDialog.

It was doing nothing before.

This avoids an additional level of nesting that probably doesn't make much sense to users (as suggested in review).

This will allow users to easily understand where they need to go to set global configurations.

That allows to correctly delete confs that haven't been saved yet.

ccordoba12 · 2024-06-11T02:48:22Z

Thanks @dalthviz for the additional review! About your comments:

Cloning/duplicationg a global config and then removing the selected config removes all the created global configs

Good catch! It should be fixed now.

Create a new local config, open the dialog again, try to modify the config without changing the name lineedit. The circular red x appears.

Open the run dialog and choose a different preset or even just open and try to close it via the Ok button the dialog will be kept open due to the name

So my guess is that it is not possible to override an existing local config?

These problems were an oversight on my side. The thing is I added a validation so that users can't introduce repeated config names in the run dialog. But I forgot to skip it when the config name and the one shown in the preset combobox are the same. So, please test again.

Select a config from the presets and use it without having to do any modification (without having to deal with the Custom configuration section)

This should be fixed now.

Be able to select a preset and change a specific config without having to create a new one

This is not possible anymore, i.e. with the changes done here. I think it was before, but then it gave users the odd impression that settings were not saved (see issue #20700, which was reported several times).

However, to not force users to provide a name when they customize global parameters, @conradolandia and I decided it'd be a good idea to set Default (custom) as the config name in that case. If users generate a custom config and then go back to Default to change it again, we set Default (custom 1) as the configuration name, and so on.

Be able to select a preset and use it as a base for a new config with a different name

This should be possible now. Please check again.

- To do that we automatically set a custom name for global configs. That way users will be able to customize those configs as many times as they want. - Don't run config name validations for global configs if they haven't been customized.

dalthviz · 2024-06-17T21:57:51Z

Gave this another check and seems like things are now working as expected. Will give the code another check but for the most part I would say that this looks good to me 👍

dalthviz

Thanks @ccordoba12 ! Left a couple of questions/comments but this LGTM 👍

Also, on a last manual check, I noticed that seems like you can create a global config with the same name of an already create per file config:

Not sure if this could be thought as a bug or not but letting you know. I'm approving this so feel free to merge if you think is ready @ccordoba12

spyder/app/tests/test_mainwindow.py

spyder/app/utils.py

dalthviz · 2024-06-17T22:10:21Z

spyder/app/utils.py

+    #   height or position) that other widgets can need to position relative to
+    #   it.
+    # * **DO NOT** use it to access other plugins functionality through it.
+    app.main_window = main


Rather than giving access to the whole main window shouldn't be created specific accessors? So, leaving this as app._main_window = main and creating a set of functions to get the values needed so get_mainwindow_width, get_mainwindow_height, get_mainwindow_position, etc?

That's a very good idea! Thanks @dalthviz for the suggestion 👍🏽

I'll implement it before merging.

dalthviz · 2024-06-17T22:10:59Z

spyder/config/main.py

@@ -643,7 +643,6 @@
        ('run', [
            'breakpoints',
            'configurations',
-            'defaultconfiguration',


There is no need to bump the config version, right?

This config option wasn't used anywhere. That's why I didn't bump CONF_VERSION.

spyder/plugins/run/container.py

Co-authored-by: Daniel Althviz Moré <16781833+dalthviz@users.noreply.github.com>

ccordoba12 · 2024-06-17T23:16:28Z

Also, on a last manual check, I noticed that seems like you can create a global config with the same name of an already create per file config

Yep, that's correct. Since global configs take precedence over file ones, I think that validation is not appropriate and so I didn't implement it. Furthermore, it could be too restrictive for users given that they can create as many file configs as they want for as many files as possible.

The good thing is that there is a subtle clue in RunDialog about which configs are global and which ones belong to the file: the Delete button. If it's enabled, it means we're dealing with a file config; if not, it means the config is global.

If users let us know that's not enough, we can think about how to disambiguate them. But for now, I think that's ok.

Also, use those accessors in RunDialog to center it when uncollapsing the custom config widget.

ccordoba12 · 2024-06-18T16:29:35Z

One last note: this PR also contains a couple of small fixes to our tests to deal with the Numpy 2.0 release (which is causing the failures in master).

ccordoba12 added 30 commits December 10, 2023 10:32

Run: Hide file combobox in RunDialog

e73518d

- Instead, show label with the current file name. - That helps to decrease the complexity of this dialog.

Run: Remove checkbox to always show RunDialog

5e0182f

This also helps to decrease the complexity of that dialog.

Run: Simplify layout of executor and parameters comboboxes in RunDialog

2c33b79

Run: Fix deleting parameters from its config page

bdbef37

Also, disable delete and clone buttons if there's no index selected in RunParametersTableView and add a test for this new functionality.

Widgets: Make HoverRowsTableView highlight a hovered row on its own

1dcf7d5

Run: Make RunParametersTableView highlight its rows on hover

448cc85

- Also, improve the main label of its config page and fix the name of one of its actions.

Run: Fix saving config parameters when the user has saved some before

db398f7

Also add test to cover that case.

Widgets: Add a new CollapsibleWidget to hide and show child widgets

f57ba7b

- This widget is based on superqt's QCollapsible widget. - It has several style customizations on top to match our style.

Run: Group widgets to customize config into a collapsible one in RunD…

51063c8

…ialog - Also, move QLineEdit to name the config to the top and show it inside a QGroupBox. - And move extra spacing between QGroupBoxes to the style of the Preferences dialog.

Minor fixes to the run config group of several plugins

530bc9a

- IPython console group: Remove grid layout to give more space to introduce command line options. - External terminal: Change text of of several widgets to make options easier to understand. - Profiler: Minor inheritance problems.

Run: Center dialog after custom_config widget is expanded/collapsed

3a48a87

Merge branch 'master' into improve-run-config-ux

8be23b4

Widgets: Change cursor shape when hovering toggle button of Collapsib…

f5cc47c

…leWidget - We use PointingHandCursor for that, which will better indicate to users that the button is clickable. - That requires a more recent version of superqt, which exposes that button publicly.

Run: Several fixes to its widgets

ebf949f

- Correctly display the entries of its comboboxes now that they are instances of SpyderComboBox. - Fix error in Python 3.10+ when centering RunDialog. - Fix error in its config page when an executor doesn't have parameters for specific files.

IPython console: Rename file that contains run conf options to run_conf

543ad31

This is done to follow the same convention present in the Profiler and External Terminal plugins.

Set min width for command line options of run conf widgets

5c00548

This will give more space to users to enter them.

Run: Correctly set bottom margins for QGroupBoxes shown in its dialogs

03adbaf

The previous solution was not correct because it cropped the title of those QGroupBoxes for some font types.

Run: Allow to delete and save globally config parameters in RunDialog

f0a0aba

Editor: Remove unnecessary run extensions

2706ea3

Those extensions are correctly added by other plugins.

Simplify code related to Run options in plugins to make it easier to …

4e07ae5

…read

Run: Don't allow to remove default executor parameters in its confpage

e22e6c9

Run: Add "Edit selected" button to its confpage

3b5b773

That way it'll be easier for users to understand that they can edit even the default configurations.

Run: Improve ExecutionParametersDialog UI when displaying default params

ddb8541

- Use localized version of "Default" string instead of the params name. - Don't allow to change the params name.

Run: Improve code and UX related to reset changes in its confpage

74d45fb

Run: Remove "Save globally" button and move "Delete" one to button box

ffc08a5

- This helps to simplify the dialog and avoid some serious UX issues found in review. - Also, enable the delete button only for file configurations and fix tests that were affected by this change.

ccordoba12 force-pushed the improve-run-config-ux branch from 81744d7 to b193699 Compare June 10, 2024 00:08

ccordoba12 added 8 commits June 10, 2024 19:11

Run: Don't allow to save file parameters with a repeated name

9e87ea5

Run: Improve validation of global configuration names

a6eb582

- Show error status icon when name is empty or already taken. - Disable changing the config name if it's a saved one. - Show the same messages in both ExecutionParametersDialog and RunDialog.

Run: Fix disabling edit/delete/clone buttons after some operations

06e35bd

Run: Fix Reset button in ExecutionParametersDialog

5598cf1

It was doing nothing before.

Run: Remove "Runner settings" groupbox to simplify its dialogs

3d85d44

This avoids an additional level of nesting that probably doesn't make much sense to users (as suggested in review).

Run: Add menu entry to go to its Preferences page

b741f4e

This will allow users to easily understand where they need to go to set global configurations.

Run: Use table_model current executor params when deleting confs

554ed05

That allows to correctly delete confs that haven't been saved yet.

ccordoba12 force-pushed the improve-run-config-ux branch 2 times, most recently from e8322fb to 08ffabc Compare June 11, 2024 02:38

ccordoba12 force-pushed the improve-run-config-ux branch from 08ffabc to 353e4bd Compare June 14, 2024 17:47

dalthviz approved these changes Jun 17, 2024

View reviewed changes

dalthviz mentioned this pull request Jun 17, 2024

Spyder 6.0 beta2 release #22176

Closed

Apply suggestions from code review

ef830c7

Co-authored-by: Daniel Althviz Moré <16781833+dalthviz@users.noreply.github.com>

ccordoba12 added 2 commits June 17, 2024 21:07

Utils: Add accessors to SpyderApplication for main window properties

b1e6280

Also, use those accessors in RunDialog to center it when uncollapsing the custom config widget.

Testing: Fix failing tests due to the Numpy 2.0 release

5e801e3

ccordoba12 merged commit 14f8853 into spyder-ide:master Jun 18, 2024
23 checks passed

ccordoba12 deleted the improve-run-config-ux branch June 18, 2024 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Improve UI/UX of the Run plugin configuration widgets #22141

PR: Improve UI/UX of the Run plugin configuration widgets #22141

ccordoba12 commented Jun 3, 2024 •

edited

Loading

dalthviz commented Jun 6, 2024

jitseniesen commented Jun 7, 2024

ccordoba12 commented Jun 10, 2024 •

edited

Loading

ccordoba12 commented Jun 10, 2024

dalthviz commented Jun 10, 2024

ccordoba12 commented Jun 11, 2024 •

edited

Loading

dalthviz commented Jun 17, 2024

dalthviz left a comment

dalthviz Jun 17, 2024

ccordoba12 Jun 17, 2024

dalthviz Jun 17, 2024

ccordoba12 Jun 17, 2024

ccordoba12 commented Jun 17, 2024 •

edited

Loading

ccordoba12 commented Jun 18, 2024

PR: Improve UI/UX of the Run plugin configuration widgets #22141

PR: Improve UI/UX of the Run plugin configuration widgets #22141

Conversation

ccordoba12 commented Jun 3, 2024 • edited Loading

Description of Changes

Visual changes

Configuration per file

Preferences page

Dialog to create global run configurations

Issue(s) Resolved

Affirmation

dalthviz commented Jun 6, 2024

jitseniesen commented Jun 7, 2024

ccordoba12 commented Jun 10, 2024 • edited Loading

ccordoba12 commented Jun 10, 2024

dalthviz commented Jun 10, 2024

ccordoba12 commented Jun 11, 2024 • edited Loading

dalthviz commented Jun 17, 2024

dalthviz left a comment

Choose a reason for hiding this comment

dalthviz Jun 17, 2024

Choose a reason for hiding this comment

ccordoba12 Jun 17, 2024

Choose a reason for hiding this comment

dalthviz Jun 17, 2024

Choose a reason for hiding this comment

ccordoba12 Jun 17, 2024

Choose a reason for hiding this comment

ccordoba12 commented Jun 17, 2024 • edited Loading

ccordoba12 commented Jun 18, 2024

ccordoba12 commented Jun 3, 2024 •

edited

Loading

ccordoba12 commented Jun 10, 2024 •

edited

Loading

ccordoba12 commented Jun 11, 2024 •

edited

Loading

ccordoba12 commented Jun 17, 2024 •

edited

Loading