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

PR: Improve UI of PaneEmptyWidget, show message on panes connected to dead consoles and improve About dialog UI #21134

Merged
merged 14 commits into from
Jul 28, 2023

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Jul 14, 2023

Description of Changes

  • For PaneEmptyWidgets: show description at the bottom and increase size of main text (these changes were suggested by @conradolandia); and load their icons in a way that doesn't make them appear pixelated at scale factors greater than one.

    imagen

  • For dead consoles: show an appropriate message using a PaneEmptyWidget on panes that depend on the console to work. This needs an additional icon that @conradolandia is going to design and add in a follow-up PR.

    imagen

  • This also includes some improvements to the About dialog: use font sizes relative to the interface one, correctly align buttons to the right, increase font size for its tabs, remove white pixels below tabs and make it bigger.

    Before

    imagen

    After

    imagen

  • Fix color of links in Qt widgets, which was broken in PR: Make the font used by the application configurable and other UI fixes #20933.

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

@ccordoba12
Copy link
Member Author

@dalthviz, could you test that the changes I did to the About dialog work as expected on Windows and Mac? Thanks!

@ccordoba12 ccordoba12 changed the title PR: Improve UI of PaneEmptyWidget and show message using that widget on panes connected to dead consoles PR: Improve UI of PaneEmptyWidget and show message on panes connected to dead consoles Jul 14, 2023
@dalthviz
Copy link
Member

This what I'm seeing on Windows:

image
image
image

On macOS:

image
image
image

@ccordoba12
Copy link
Member Author

Thanks @dalthviz! I think the dialog looks good on Windows but I'm not sure about Mac.

Could you post a single screenshot per OS of how things look in master? Thanks!

@dalthviz
Copy link
Member

@ccordoba12 using master I got this:

  • On Windows:

image

  • On macOS:

image

@ccordoba12
Copy link
Member Author

@ccordoba12 using master I got this

Ok, thanks for checking! I'll make the necessary adjustments on Windows so that things look the same.

But could you give me a hand in Mac? Please use different values in DialogStyle until you find the ones that match with master so I can make the necessary changes here.

@ccordoba12
Copy link
Member Author

But could you give me a hand in Mac? Please use different values in DialogStyle until you find the ones that match with master so I can make the necessary changes here.

Or better yet, just give me the default interface font size on Mac and I'll adjust the DialogStyle constants according to it (that's what I did on Windows).

@dalthviz
Copy link
Member

Or better yet, just give me the default interface font size on Mac and I'll adjust the DialogStyle constants according to it (that's what I did on Windows).

Is this what you need?:

image

Or is there some other place where macOS defines that @ccordoba12 ?

@ccordoba12
Copy link
Member Author

Yep, that's it. Thanks @dalthviz! I'll make the necessary changes right away.

@ccordoba12
Copy link
Member Author

The About dialog should have the same font sizes again on Windows and Mac, but please check on Mac to be sure (I already did on Windows).

@dalthviz
Copy link
Member

This is what I'm seeing now on macOS:

image

@ccordoba12
Copy link
Member Author

Ok, it looks as in master, which is good. So, that's working fine now.

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 ! Left a comment regarding the TODO about the icon to use with the errored console.

Also, while checking I noticed that there is an issue with the Variable Explorer when it is not connected to a console since the toolbar buttons are still enabled and you can't open the hamburger menu. Trying to interact with those you get errors like:

  • Interacting with the hamburger menu:
Traceback (most recent call last):
  File "e:\acer\documentos\spyder\spyder otros\carlos\spyder\spyder\api\widgets\main_widget.py", line 441, in _update_actions
    self.update_actions()
  File "e:\acer\documentos\spyder\spyder otros\carlos\spyder\spyder\plugins\variableexplorer\widgets\main_widget.py", line 429, in update_actions
    save_data_action.setEnabled(nsb.filename is not None)
AttributeError: 'PaneEmptyWidget' object has no attribute 'filename'
  • Interacting with the Import data:
Traceback (most recent call last):
  File "e:\acer\documentos\spyder\spyder otros\carlos\spyder\spyder\plugins\variableexplorer\widgets\main_widget.py", line 181, in <lambda>
    triggered=lambda x: self.import_data(),
  File "e:\acer\documentos\spyder\spyder otros\carlos\spyder\spyder\plugins\variableexplorer\widgets\main_widget.py", line 488, in import_data
    nsb.refresh_table()
AttributeError: 'PaneEmptyWidget' object has no attribute 'refresh_table'

Probably when detecting the console errored, besides the new message, we should also disable the Variable Explorer buttons and review the update_actions logic to not trigger logic that could need the current namespacebrowser widget?

spyder/api/shellconnect/main_widget.py Show resolved Hide resolved
@ccordoba12
Copy link
Member Author

Also, while checking I noticed that there is an issue with the Variable Explorer when it is not connected to a console since the toolbar buttons are still enabled and you can't open the hamburger menu

Thanks for checking @dalthviz! I went through all buttons and menu entries in the Variable Explorer, Plots and Debugger and fixed the errors I found related to these changes.

So, please test again.

@ccordoba12 ccordoba12 force-pushed the empty-panes-improvements branch 2 times, most recently from 60cb321 to 0eff218 Compare July 21, 2023 17:38
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 ! Left a couple of comments about the order of the validations for the empty widget vs the line where the current widget gets referenced. Besides that, could it be worthy to disable the buttons in the Variable Explorer? Now there are no errors, but is kind of strange to be able to push buttons or toggle them:

not_connected

By disabling the buttons the behavior would be closer to the Plots pane which seems logic to me (if a functionality that a button provides is not available the button gets disabled):

image

The debugger partially does the same thing (except for the search button):

image

spyder/plugins/debugger/widgets/main_widget.py Outdated Show resolved Hide resolved
spyder/plugins/variableexplorer/widgets/main_widget.py Outdated Show resolved Hide resolved
- Make the text label bigger.
- Use the interface font size for the description label.
- Make description label appear at the bottom.
- This is useful to display a meaningful message in plugins whose
content depends on being connected to an active console.
- We use PaneEmptyWidget for the message.
This way ShellConnectMixin can be used for objects that are not plugins
(like the MatplotlibStatus widget) and the API is clearer for external
developers.
This is achieved by:
- Preserving the aspect ratio of the svg corresponding to the file.
- Scaling it according to the factor declared by users in Preferences.
- Correctly align buttons to the right of the tab pages.
- Make tabs font size match the one used for content.
- Remove white area at the bottom of unselected tabs.
…mpty

Also, introduce a new method to ShellConnectMainWidget to easily
check if the current widget is an empty one.
@ccordoba12
Copy link
Member Author

Besides that, could it be worthy to disable the buttons in the Variable Explorer? Now there are no errors, but is kind of strange to be able to push buttons or toggle them

The debugger partially does the same thing (except for the search button):

Great suggestion @dalthviz! I implemented that in my last commit.

@ccordoba12 ccordoba12 changed the title PR: Improve UI of PaneEmptyWidget and show message on panes connected to dead consoles PR: Improve UI of PaneEmptyWidget, show message on panes connected to dead consoles and improve About dialog UI Jul 25, 2023
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 ! This LGTM 👍

But, just in case, this is how the Variable Explorer buttons look when no console is available:

image

Seeing that for a bit, It's kind of difficult to distinguish the disabled filter button 🤔 Not sure at all if there is something to do there but letting you know.

I'm going to approve this but what do you think about the way the filter button looks? Feel free to merge this or ping me to merge if the way the filter button looks is okay for you 👍

@ccordoba12
Copy link
Member Author

ccordoba12 commented Jul 26, 2023

Seeing that for a bit, It's kind of difficult to distinguish the disabled filter button 🤔 Not sure at all if there is something to do there but letting you know.

Thanks @dalthviz! I'm testing things in the light theme and there the disabled and checked button doesn't look bad. But I'll take a look at it because in the dark theme that definitely doesn't work, as you pointed out.

- Make it bigger.
- Increse size of the "Spyder IDE" text in the Overview tab and fix
spacing on it.
- Remove repeated "Spyder IDE" text below the icon.
@ccordoba12
Copy link
Member Author

@dalthviz, I think I fixed the last issue you found, so please test again.

I also made some additional changes to the About dialog agreed on with @conradolandia and @CAM-Gerlach (see the OP).

@jitseniesen
Copy link
Member

One thing I noticed when I recorded the video in #21145 (comment) is that the minimum size of an empty pane is quite big, certainly larger than when the pane is not empty. It is not possible to make the Variable Explorer pane any smaller at the start of the video, when the pane is empty,

@ccordoba12
Copy link
Member Author

One thing I noticed when I recorded the video in #21145 (comment) is that the minimum size of an empty pane is quite big

Thanks for pointing that our @jitseniesen. Since this is almost ready and @conradolandia needs it to do more improvements on top of it, I think we could merge it and address your concern later.

So, could you open a new issue about it?

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @ccordoba12 !

@ccordoba12 ccordoba12 merged commit 2540935 into spyder-ide:master Jul 28, 2023
22 checks passed
@ccordoba12 ccordoba12 deleted the empty-panes-improvements branch July 28, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants