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 style of dockwidget tabbars (UI) #21133

Merged
merged 10 commits into from
Jul 28, 2023

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Jul 14, 2023

Description of Changes

Dark theme

new-tabbar-on-dark-theme

Light theme

new-tabbar-on-light-theme

Issue(s) Resolved

Fixes spyder-ide/ux-improvements#4.

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 ccordoba12 added this to the v6.0alpha2 milestone Jul 14, 2023
@ccordoba12 ccordoba12 requested a review from dalthviz July 14, 2023 01:28
@ccordoba12 ccordoba12 self-assigned this Jul 14, 2023
@ccordoba12 ccordoba12 changed the title PR: Improve style of dockwidget tabbars PR: Improve style of dockwidget tabbars (UI) Jul 14, 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 ! Checking on Windows and seems like is looking as previewed 👍

The only thing that I noticed is that the scroll buttons for the tabs are now bigger. Is that the wanted look, right?:

Before:

image

After:

image

Also, just in case, for me the scroll buttons for the pane tabs look like:

image

Edit: Remembered that there is a setting to change the tabs from horizontal to vertical and checking seems like the custom style here doesn't support it:

image

@ccordoba12
Copy link
Member Author

ccordoba12 commented Jul 18, 2023

@dalthviz, thanks for the review! I think I fixed the errors you found, so please test again.

I also made these additional changes:

  • I revisited margins for all panes to make them work as expected with the new style.
  • I asked for a restart to change the vertical tabs option because it now requires to adjust all PluginMainWidget margins, and that can't be done on the fly.
  • I fixed an error when closing Spyder (related to the Working directory) that appeared from time to time.

Also, introduce a constant for this and remove hard-coded values
- That margin is not necessary anymore with the new dock tabbar style.
- Also improve its placement in the IPython console.
@dalthviz
Copy link
Member

Checking this again seems like the tab arrows are now a little bit too small:

image
image

However, now the style adapts to the vertical layout 👍

image

Copy link
Member Author

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Checking this again seems like the tab arrows are now a little bit too small

Ok, I adjusted that size to be similar to the one I'm seeing on Linux. But the nice thing is that it can be changed to be a bit bigger (which can't be done on Linux).

Below I left a comment in the places where I set those values, so please change them and let me know which ones work better for you.

spyder/utils/stylesheet.py Outdated Show resolved Hide resolved
spyder/utils/stylesheet.py Outdated Show resolved Hide resolved
@ccordoba12
Copy link
Member Author

@dalthviz, this should be ready, unless you have more comments about the code.

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 for all the work here! I left a question regarding the line commented as a hack but otherwise this LGTM 👍

Comment on lines +372 to +376
# TODO: This is a hack! Remove it after migrating to the new API
self.find_widget.layout().setContentsMargins(
2 * MARGIN_SIZE, MARGIN_SIZE, 2 * MARGIN_SIZE, MARGIN_SIZE
)

Copy link
Member

Choose a reason for hiding this comment

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

Why this was needed? Are there other changes that need to be done besides removing this line for the migration?

Copy link
Member Author

@ccordoba12 ccordoba12 Jul 28, 2023

Choose a reason for hiding this comment

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

Why this was needed?

Because in this PR I added a bottom margin to all migrated plugins here, so that their borders don't touch the bottom. And by doing that, it was not necessary to add a bottom margin to the Find/Replace widget, so I removed it here.

But since the editor is not migrated, we need to add that margin in the meantime (which is the change you highlighted here). Otherwise the Find/Replace widget would appear touching the bottom.

Are there other changes that need to be done besides removing this line for the migration?

Not from this PR, but I've been documenting other things that need to be removed after the migration with TODOs in spyder/plugins/editor/plugin.py.

@ccordoba12 ccordoba12 merged commit 2ea17b7 into spyder-ide:master Jul 28, 2023
@ccordoba12 ccordoba12 deleted the dock-tabbar-style branch July 28, 2023 00:48
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.

Change design of pane tabs for them to look different to the tabs of the console and editor
3 participants