-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 style of the Preferences dialog (UI) #21233
PR: Improve style of the Preferences dialog (UI) #21233
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ccordoba12 ! Checked this locally and I noticed a couple of behaviors that maybe are not wanted:
- It is possible to set a minimum height for the preferences dialog that seems like quite small:
- Seems like the
Appearance
page triggers an horizontal scrollbar:
- A small enough dialog height causes some pages to show labels cutted off (
Application
andEditor > Advanced settings
):
Also, thinking about this style, should the About dialog also use the new tabbar style and border removal idea? Comparing the dialogs side by side:
f86d9e9
to
fb661b6
Compare
That way we avoid them to take the entire width of the config dialog.
Also, remove dead code related to that
- Add background color to the entries area to visually distinguish it from the options one. - Increase entries font size in one point. - Remove border from and add padding to options area.
Introduce a new SpecialTabBarStyleSheet class for that.
- Break ungrouped options into groups so users can tell them apart more easily. - Improve names of some options and add tooltips to others.
- Make consistent the use of "Interface" and "Display" with other plugins. - Add a new section about "Confirmations" in the "Interface" tab.
That's because that module was not really exposing an API but just declaring several widgets.
fb661b6
to
ebdf18d
Compare
The most important sections are shown first, then the other plugin pages are shown alphabetically and finally we show the Plugins page.
Also, reduce the size of that area.
This happened only when the table has no widgets
- It seems to not be necessary anymore. - Besides, it was being called on enter and leave events, which was a waste of resources.
ebdf18d
to
3fde5e9
Compare
Hello @ccordoba12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2023-11-04 14:30:53 UTC |
- This is necessary for Mac. - Also, fix style of tabbar scroll buttons on Windows and Mac.
3fde5e9
to
175953d
Compare
1d83832
to
e37a538
Compare
- This prevents text from being clipped when there's not enough space available. - Also, fix similar error in the editor config page.
e37a538
to
9935686
Compare
Also, improve description of some dependencies
Also, use signals_blocked utility of superqt to simplify a bit of code.
This is only necessary on Mac.
5f50102
to
a6aa3c1
Compare
- Set default and min sizes for its dialog. - Increase items padding on Linux. - Decrease contents area width on Windows and increase it on Linux. - Adjust min height of editor config page to avoid clipped text.
a6aa3c1
to
66700f0
Compare
@dalthviz, this is ready for another review. About your comments above:
Well thought! I followed your recommendation and did exactly that.
Fixed by setting a min width.
Thanks for noticing it! This problem shouldn't show up anymore.
I thought exactly the same! The refactoring you suggested (i.e. creating |
Also @dalthviz, the Mac and Windows screenshots in the OP are slightly outdated. Could you give me a hand and upload new ones with my latest changes? Thanks! |
b573301
to
f739662
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ccordoba12 ! I checked locally on Windows and seems like things are working as expected. I left a comment regarding the preferences plugin widgets/API namespace change. Anyhow, since this is kind of needed for other things you're working on, I'm leaving this approved so feel free to merge if you think no further changes are needed 👍
from spyder.plugins.preferences.widgets.config_widgets import ( | ||
SpyderConfigPage, | ||
BaseConfigTab | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we preserve the import call that was before and instead do this new import from spyder.plugins.preferences.api
? Although those definitions are widgets, it is also true that they are part of the preferences plugin API, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although those definitions are widgets, it is also true that they are part of the preferences plugin API, no?
I'd say that not really. I mean, I think what we should put in spyder/plugins/myplugin/api.py
are things that other plugins need to import to work in relationship to myplugin
. That's why we have mainly enums for actions in the api
module of most of our plugins (to place other actions in menus or toolbars before or after them, for instance).
In this particular case, other plugins don't need to import SpyderConfigPage
and BaseConfigTab
but SpyderPreferencesTab
and PluginConfigPage
, which are declared here.
However, your comment made me realize that the MOST_IMPORTANT_PAGES
constant that I added to spyder.plugins.preferences.api
shouldn't be there. That's because that constant is not needed by other plugins but by several modules in the Preferences plugin. So, I put it in api.py
to avoid circular imports, but I think it's better placed in spyder.plugins.preferences.__init__
, which is where I'll move it in my next commit.
- That's because that constant doesn't need to be imported by other plugins but shared by several modules in this one. - So, we need a separate module to declare it in order to avoid circular imports.
* Return value of `.create_checkbox()` changed in spyder-ide/spyder#20926 (Show tooltips in Preferences). * Handling of Prefs dialog box size changed in spyder-ide/spyder#21233 (Improve style of Preferences dialog box).
Description of Changes
Plugins
page last.superqt
as a new dependency to use some of the utilities declared on it.Before
After
macOS
Windows
Linux
Issue(s) Resolved
Fixes #15073.
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