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

Light (Preview) Font color change for text in tabs #2294

Merged

Conversation

mvm-sap
Copy link
Contributor

@mvm-sap mvm-sap commented Sep 18, 2024

Font color of all tabs selected or unselected are set to black #000000 with this change to have a better contrast.

Before :
image

After:
image

@BeckerWdf
Copy link
Contributor

how is this different to #2187 ?

@mvm-sap
Copy link
Contributor Author

mvm-sap commented Sep 18, 2024

how is this different to #2187 ?

In #2187
Unselected Tabs in view and editor had #3b3b3b
and
Selected Tabs in view and editor had #000000

But with this PR
All tabs, selected and unselected both in editor and view has #000000

We need to delete #2187

Copy link
Contributor

github-actions bot commented Sep 18, 2024

Test Results

0 files   -  1 815  0 suites   - 1 815   0s ⏱️ - 1h 42m 51s
0 tests  -  7 699  0 ✅  -  7 471  0 💤  - 228  0 ❌ ±0 
0 runs   - 24 258  0 ✅  - 23 511  0 💤  - 747  0 ❌ ±0 

Results for commit 7c3154a. ± Comparison against base commit 88b1d36.

♻️ This comment has been updated with latest results.

@BeckerWdf
Copy link
Contributor

looks good on macOS for me.
@jukzi: Can you pls. also test and say if this addresses your issues?

@jukzi
Copy link
Contributor

jukzi commented Sep 18, 2024

looks better, but it's hard to see which secondary tab is active:
image
used to look like this where it was obvious which tab is the active view by background color:
image

@BeckerWdf
Copy link
Contributor

but it's hard to see which secondary tab is active

Do you mean the "Console" tab in your screenshot? It's now indicated by the grey underline.

@jukzi
Copy link
Contributor

jukzi commented Sep 18, 2024

Do you mean the "Console" tab in your screenshot? It's now indicated by the grey underline.

yes. Unfortunately that grey underline is not as prominent as the changed background. isn't there even some option to diable that underline? Then it would be totally unclear.
Maybe i can get used to the underline, but at a first glance the old style was more expressive.

@BeckerWdf
Copy link
Contributor

Do you mean the "Console" tab in your screenshot? It's now indicated by the grey underline.

yes. Unfortunately that grey underline is not as prominent as the changed background. isn't there even some option to diable that underline? Then it would be totally unclear. Maybe i can get used to the underline, but at a first glance the old style was more expressive.

Yes that's true but that's the intention of that change. As it's not the "primary active tab" but only the "secondary active tab" is should be not as prominent. We want to give the "main actor" more focus and the "secondary active tab" is not only a "supporting actor". But we could make the grey underline a bit darker grey to make it a bit more prominent but not too prominent.
What about something like this:
Screenshot 2024-09-19 at 09 39 27
compared to the current state:
Screenshot 2024-09-19 at 09 40 53

@jukzi
Copy link
Contributor

jukzi commented Sep 19, 2024

darker looks better. maybe even one pixel thicker?

@jukzi
Copy link
Contributor

jukzi commented Sep 19, 2024

The best would be if the high contrast icons of the inactive tabs gets reduced :-) just as idea

@BeckerWdf
Copy link
Contributor

The best would be if the high contrast icons of the inactive tabs gets reduced

That's a nice idea but currently not possible to implement. We could save this for a further improvement via a separate PR.

@BeckerWdf
Copy link
Contributor

maybe even one pixel thicker?

We just made the line thinner in the last release. This is something we can improve (and maybe make it bigger again) later on.

@BeckerWdf BeckerWdf force-pushed the LightPrev_Text_Color_Change_For_Tabs branch from 7a9c17d to 874d652 Compare September 19, 2024 11:04
@BeckerWdf
Copy link
Contributor

BeckerWdf commented Sep 19, 2024

darker looks better. maybe even one pixel thicker?

I just pushed a second commit into this pr making the grey darker.
Now looks like the one on the right side. Left side is the current grey color:
image

mvm-sap and others added 2 commits September 19, 2024 13:58
Font color of all tabs selected or unselected are set to black  #000000
with this change
To make it easier to see which one is the "secondary active tab"
(the active tab in inactive part stacks) the grey line in that tab
is made a bit darker.
@BeckerWdf BeckerWdf force-pushed the LightPrev_Text_Color_Change_For_Tabs branch from 874d652 to 7c3154a Compare September 19, 2024 11:58
@BeckerWdf
Copy link
Contributor

https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/PR-2294/4/console reports a lot of API breakages.
They are not caused by this PR and for it looks like false alarms.
Does anybody have a clue what's happening?

@jukzi
Copy link
Contributor

jukzi commented Sep 19, 2024

Does anybody have a clue what's happening?

may be a random fail retry.

Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

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

LGTM

@BeckerWdf BeckerWdf merged commit 8df2017 into eclipse-platform:master Sep 20, 2024
7 of 12 checks passed
@mvm-sap mvm-sap deleted the LightPrev_Text_Color_Change_For_Tabs branch September 23, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants