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: Change style of border around thumbnail of current plot #21598

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

jitseniesen
Copy link
Member

Description of Changes

  • Wrote at least one-line docstrings (for any new functions) N/A
  • Added unit test(s) covering the changes (if testable) N/A
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Change the border around the thumbnail of the current plot in the Plots pane to 3px wide and gray (COLOR_TEXT_3) for clarity and consistency with the overall styling. This style was suggested in this comment.

Screenshot after the change in dark theme:

plots-pr-dark

Screenshot after the change in light theme:

plots-pr-light

Issue(s) Resolved

Fixes #21046

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: Jitse Niesen

@ccordoba12
Copy link
Member

ccordoba12 commented Dec 10, 2023

Thanks @jitseniesen for your work on this! I think the new border style looks quite good in the light theme but not so much in the dark one because it blends a bit with the plot's own color.

@conradolandia, what do you think?

@conradolandia
Copy link
Contributor

I agree with you, @ccordoba12. Let's try B70 (from the Gray class):
image

@jitseniesen
Copy link
Member Author

This is what it looks like with Gray.B70 in dark mode:

plots-pr2-dark

@conradolandia
Copy link
Contributor

I am thinking we should go back to the blue. Preserving the line thickness, but going back to the blue color we had before. That will look good in both the dark and light theme.

@jitseniesen
Copy link
Member Author

I am not certain that my previous screenshot was correct. This is what it really looks like with Gray.B70:

plots-pr3-dark

This is the original blue color, but with 3px thickness instead of the original 2px thickness. I think it is clearer than before and thus an improvement.

plots-pr4-dark

Same in light mode:

plots-pr4-light

@jitseniesen jitseniesen marked this pull request as draft December 16, 2023 21:24
@ccordoba12
Copy link
Member

This is the original blue color, but with 3px thickness instead of the original 2px thickness. I think it is clearer than before and thus an improvement.

I think so too. So, I propose that we should go with this option instead.

@conradolandia, what do you think?

@conradolandia
Copy link
Contributor

I think this option works fine enough, yes.

@jitseniesen jitseniesen marked this pull request as ready for review January 2, 2024 19:13
@jitseniesen
Copy link
Member Author

Great, I squashed the commits in one, as it is basically a one-character change now.

@ccordoba12 ccordoba12 modified the milestones: v6.0alphaX, v6.0beta1 Jan 2, 2024
Copy link
Member

@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.

Thanks @jitseniesen!

Note: The error in our test suite is unrelated to this.

@ccordoba12 ccordoba12 merged commit ee062e4 into spyder-ide:master Jan 3, 2024
13 of 14 checks passed
@jmshon-ssu
Copy link

jmshon-ssu commented Jan 3, 2024 via email

@jitseniesen jitseniesen deleted the plot-highlight branch January 3, 2024 10:39
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.

Improve highlighting of current plot
4 participants